peloton icon indicating copy to clipboard operation
peloton copied to clipboard

Clean-up Catalog Infrastructure

Open apavlo opened this issue 7 years ago • 5 comments

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 avatar Jun 10 '18 19:06 apavlo

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

tli2 avatar Jun 11 '18 15:06 tli2

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

apavlo avatar Jun 11 '18 16:06 apavlo

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

tli2 avatar Jun 18 '18 14:06 tli2

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

tli2 avatar Jun 19 '18 14:06 tli2

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.

tli2 avatar Jun 27 '18 16:06 tli2