search

cmu-db / peloton

The Self-Driving Database Management System
http://pelotondb.io
Apache License 2.0
2.03k stars 624 forks source link

Clean-up Catalog Infrastructure #1398

Open apavlo opened 6 years ago

apavlo commented 6 years ago

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.

tli2 commented 6 years ago

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 commented 6 years ago

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

tli2 commented 6 years ago

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

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

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.