Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Clean-up Catalog Infrastructure #1398

Open
apavlo opened this issue Jun 10, 2018 · 5 comments
Open

Clean-up Catalog Infrastructure #1398

apavlo opened this issue Jun 10, 2018 · 5 comments
Labels

Comments

@apavlo
Copy link
Member

apavlo commented Jun 10, 2018

Our catalog code is a mess. It's inconsistent. We need to clean it up so that it is easier to understand and follows the proper logical hierarchy.

I propose the following changes:

  1. Refactor the methods so that TransactionContext is always passed in as the first argument.

  2. Refactor the Catalog methods so that parameters are passed in using the same order as the hierarchy (i.e., Database→Schema→Table). I think that Indexes should be under Table as well. Right now they are under Databases.

  3. DatabaseCatalogObject should not allow you to get TableCatalogObject. DatabaseCatalogObject should only allow you to get SchemaCatalogObject and then all of the tables should be stored in SchemaCatalogObject instead.

  4. A more controversial move would be to rename SchemaCatalogObject to NamespaceCatalogObject. Postgres exposes this as pg_namespace. I think we should switch.

@apavlo apavlo added the cleanup label Jun 10, 2018
@tli2
Copy link
Contributor

tli2 commented Jun 11, 2018

Speaking of XXXCatalogObjects, what are they exactly? From what I understand they are results from a read. Would it make more sense to rename them to "CatalogEntry" or something like that? I remember getting confused by it while doing class project and not being able to find much documentation explaining what it is

@apavlo
Copy link
Member Author

apavlo commented Jun 11, 2018

@tli2 I am okay with renaming them to be CatalogEntry.

@tli2
Copy link
Contributor

tli2 commented Jun 18, 2018

Since I am blocked on a weird test failure in #1404, I can quickly fix some of these today.

@tli2 tli2 self-assigned this Jun 18, 2018
@tli2 tli2 removed their assignment Jun 18, 2018
@tli2
Copy link
Contributor

tli2 commented Jun 19, 2018

  1. Remove all default parameters (or document them clearly for the ones that really make sense), per Catalog code cleanup #1414

@tli2
Copy link
Contributor

tli2 commented Jun 27, 2018

As @pervazea pointed out, we are a long way from done on this one, as there are multiple naming issues and other inconsistencies in the code. To make matters worse very little documentation is provided so it is unclear what any of the API should behave.

ksaito7 added a commit to ksaito7/peloton that referenced this issue Jun 28, 2018
ksaito7 added a commit to ksaito7/peloton that referenced this issue Jun 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants