Remove the call to the Optimizer from several catalog instance initializations

[linmagit]

We have many different global catalog singletons in our system, such as DatabaseCatalog, ZoneMapCatalog. (Ideally we want to combine those singletons together, but that's another longer discussion.)

We now have two ways to initialize a catalog instance:
https://github.com/cmu-db/peloton/blob/master/src/include/catalog/abstract_catalog.h#L48
https://github.com/cmu-db/peloton/blob/master/src/include/catalog/abstract_catalog.h#L53

The second way takes in a SQL DDL, and use the Optimizer to generate the create plan and then initialize the catalog. This is problematic because the Optimizer needs to use the information stored in the catalog to generate the plan (if implemented correctly...), but the catalogs are still under initialization here. And at the higher level, the Catalog should not depend on the implementation of the Optimizer just because this create plan generation here.

We should construct the plan/schema required by those catalog initializations by ourselves, and remove the Optimizer dependency here. This includes these catalog objects:
ColumnStatsCatalog, DatabaseMetricsCatalog, IndexMetricCatalog, LanguageCatalog, ProcCatalog, QueryMetricsCatalog, SettingsCatalog, TriggerCatalog, and ZoneMapCatalog.

Other catalog objects are using the first way to initialize which is fine.

[poojanilangekar]

I'm not sure if there is a quick fix to this.

The tables which are created in the first way in thepg_catalog are created when the catalog's constructor is called. This works fine for those particular tables since they are always created by default. These tables also have default OIDs defined in catalog_defaults.h.

However, the other tables eg. ColumnStatsCatalog are only created at runtime if necessary. Also, I think what @malin1993ml mentioned should not happen, since the pg_catalog (created when the Catalog::GetInstance function is invoked for the first time) already exists when the ColumnStatsCatalog, TriggerCatalog, etc. are being created. So the optimizer will be able to find the information it needs in the catalog.

Yet I agree that we shouldn't have a dependency between the Catalog and the Optimizer. However, since we need an existing OID to instantiate an AbstractCatalog object. I think the first step would be to provide an interface to create internal tables. We could go around with some hacks to directly provide the schema instead of the DDL but then who validates the table_name/schema of the internal tables? Currently this is only done in the constructor of the Catalog and there's no way around it because those are the first tables which are to be created. I feel like passing it through the parser and the planner ensures that the schema being created is valid.

Also I feel we need to have a clear demarcation between internal tables and user created tables. Currently, we can query these internal tables from the PSQL client, which does not sound correct to me. @apavlo What do you think?

[linmagit]

@poojanilangekar Yes, pg_catalog would already exist when the optimizer is called to create ColumnStatsCatalog, TriggerCatalog, etc. However, the optimizer may need the information in the latter catalog instances as well. E.g., the optimizer may need to know what triggers/zone maps are there to optimize the plan for a query. It's just the optimizer is not supporting those features right now, and only uses the information in the pg_catalog. Once the optimizer starts to use those information, this won't work anymore.

[poojanilangekar]

@malin1993ml I think the ZoneMaps/Triggers, etc. would never be created on startup. But instead get created when the brain calls the appropriate functions, right? So the Optimizer can always query with or without the ZoneMap/Trigger. Hence it might querypg_catalog for a ZoneMap, find that it doesn't exist and then just continue execution in the naive way. I am not sure, but I was under the impression that the Optimizer should always be able to support queries even with just the basic information.

[linmagit]

I don't know exactly how the ZoneMap works, but I thought you can only know whether a ZoneMap exists by checking the ZoneMap catalog? Can you get that from pg_catalog?

[poojanilangekar]

I think even if the Optimizer queried the ZoneMapManager, it would return false when querying to check whether or not the ZoneMap exists. Only once the CreateZoneMap function commits the transaction, it would return true. Maybe @rohit-cmu Can clear the confusion.

[rohit-agrawal10]

Pooja is right. It will return true only the way Pooja has mentioned it. I also agree with the internal table APIs and the demarcation.

…

On Sat, Jan 20, 2018 at 3:04 AM poojanilangekar ***@***.***> wrote: I think even if the Optimizer queried the ZoneMapManager, it would return false when querying to check whether or not the ZoneMap exists. Only once the CreateZoneMap function commits the translation <https://github.com/cmu-db/peloton/blob/52b28abe8e281f2c71f2042d96ac89fad74c2f11/src/storage/zone_map_manager.cpp#L56>, it would return true. Maybe @rohit-cmu <https://github.com/rohit-cmu> Can clear the confusion. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1008 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AYArDDdfEEKu7R_IkuewUhLnUTMNzDrnks5tMQppgaJpZM4RMc2o> .

[linmagit]

I see. Thanks for the clarification. Then I think the ZoneMapCatalog should be fine because we always get it from the ZoneMapManager.

However, there're some other catalog instances that are directly initialized when we bootstrap pg_catalog. Those GetInstances will eventually call the optimizer. And if the optimizer also uses those catalog to get information, such as triggers or pg_procs, I think this can cause recursive calling.

[poojanilangekar]

I looked at the call to the Bootstrap function, it is followed immediately after the call to instantiate the catalog. I think we must probably ask @AngLi-Leon @mengranwo and @camellyx for the reason behind making this call right after the constructor call. I believe they had a good reason to initialize these 'additional catalogs' (as per their comment) separately. This brings us back to needing some CatalogManager object which keeps track of all the catalogs which have and haven't been initialized. And sort of provide an interface similar to the ZoneMapManager. IMO, we should have only the bare minimum catalog tables defined in catalog_defaults.h and initialized in the constructor and everything auxiliary should be kept out of it (the way it has been done currently). What do you think?

[camellyx]

Thanks @poojanilangekar for looping us in, and you are correct, we want to minimize the catalog tables defined in catalog_defaults.h because they are created in a basic way (i.e., we have to insert records into other system catalogs by hand), which is undesirable.
And from the above discussion, I believe ColumnStatsCatalog should be created in that basic way, right? because Optimizer depends on it.
@malin1993ml Is there any other tables that Optimizer will need other than ColumnStatsCatalog? Could you also update the original issue post with the list? Thanks!

[linmagit]

@poojanilangekar I agree that we should have a CatalogManager that keeps track of all the catalogs. That would help in general no matter how we eventually decide to solve this issue.
@camellyx That's for being involved! Well, without a CatalogManager, everything would be dependent except ZoneMapCatalog which has a manager. If we have a manager, then I think it still depends:

1. For ColumnStatsCatalog: that's definitely going to be depended by the optimizer. So that should be the basic way;
2. For DatabaseMetricsCatalog, IndexMetricCatalog, QueryMetricsCatalog: I think we don't need these metrics in the optimizer. Although from the engineering perspective I think it's still better to decouple the catalog from the optimizer. But it's fine to leave them there if it's too hard.
3. For LanguageCatalog, ProcCatalog, SettingsCatalog, TriggerCatalog: In theory, the optimizer would need them as well. But we may not need them at the bootstrap stage of the catalog instances per se. It's just we need to be aware of that we cannot use the trigger(proc, etc.) functionalities when we call the optimizer in the constructors of those catalog instances, since their catalogs have not been initialized. That's probably fine since we're only using CREATE statements there, but in theory people can put anything in that SQL statement and that could cause confusion to other developers.