andr-sokolov commented on code in PR #1842: URL: https://github.com/apache/cloudberry/pull/1842#discussion_r3536652621
########## src/backend/commands/laketablecmds.c: ########## @@ -0,0 +1,483 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * laketablecmds.c + * lake table creation/manipulation commands + * + * IDENTIFICATION + * src/backend/commands/laketablecmds.c + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "access/genam.h" +#include "access/htup_details.h" +#include "access/reloptions.h" +#include "access/table.h" +#include "access/xact.h" +#include "catalog/catalog.h" +#include "catalog/dependency.h" +#include "catalog/indexing.h" +#include "catalog/objectaccess.h" +#include "catalog/pg_foreign_catalog.h" +#include "catalog/pg_foreign_volume.h" +#include "catalog/pg_lake_table.h" +#include "commands/defrem.h" +#include "commands/laketablecmds.h" +#include "foreign/foreign.h" +#include "miscadmin.h" +#include "utils/builtins.h" +#include "utils/fmgroids.h" +#include "utils/rel.h" + +/* GUC variables for default Iceberg catalog and volume */ +char *iceberg_default_catalog = NULL; +char *iceberg_default_volume = NULL; + +/* + * check_iceberg_default_catalog: validate new iceberg_default_catalog GUC value + */ +bool +check_iceberg_default_catalog(char **newval, void **extra, GucSource source) +{ + /* + * If we aren't inside a transaction, or connected to a database, we + * cannot do the catalog accesses necessary to verify the name. Must + * accept the value on faith. + */ + if (IsTransactionState() && MyDatabaseId != InvalidOid) + { + if (**newval != '\0') + { + Oid catalog_oid = get_foreign_catalog_oid(*newval, true); + + if (!OidIsValid(catalog_oid)) + { + /* + * When source == PGC_S_TEST, don't throw a hard error for a + * nonexistent catalog, only a NOTICE. See comments in guc.h. + */ + if (source == PGC_S_TEST) + { + ereport(NOTICE, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("foreign catalog \"%s\" does not exist", + *newval))); + } + else + { + GUC_check_errdetail("Foreign catalog \"%s\" does not exist.", + *newval); + return false; + } + } + } + } + + return true; +} + +/* + * check_iceberg_default_volume: validate new iceberg_default_volume GUC value + */ +bool +check_iceberg_default_volume(char **newval, void **extra, GucSource source) +{ + /* + * If we aren't inside a transaction, or connected to a database, we + * cannot do the catalog accesses necessary to verify the name. Must + * accept the value on faith. + */ + if (IsTransactionState() && MyDatabaseId != InvalidOid) + { + if (**newval != '\0') + { + Oid volume_oid = get_foreign_volume_oid(*newval, true); + + if (!OidIsValid(volume_oid)) + { + /* + * When source == PGC_S_TEST, don't throw a hard error for a + * nonexistent volume, only a NOTICE. See comments in guc.h. + */ + if (source == PGC_S_TEST) + { + ereport(NOTICE, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("foreign volume \"%s\" does not exist", + *newval))); + } + else + { + GUC_check_errdetail("Foreign volume \"%s\" does not exist.", + *newval); + return false; + } + } + } + } + + return true; +} + +/* + * GetDefaultIcebergCatalog -- get the name of the current default Iceberg catalog + * + * Returns NULL if no default catalog is set. + * This function hides the iceberg_default_catalog GUC variable. + */ +const char * +GetDefaultIcebergCatalog(void) +{ + if (iceberg_default_catalog == NULL || iceberg_default_catalog[0] == '\0') + return NULL; + + /* + * Verify that the catalog still exists. We don't cache this because + * the catalog could be dropped after the GUC was set. + */ + if (!OidIsValid(get_foreign_catalog_oid(iceberg_default_catalog, true))) + return NULL; + + return iceberg_default_catalog; +} + +/* + * GetDefaultIcebergVolume -- get the name of the current default Iceberg volume + * + * Returns NULL if no default volume is set. + * This function hides the iceberg_default_volume GUC variable. + */ +const char * +GetDefaultIcebergVolume(void) +{ + if (iceberg_default_volume == NULL || iceberg_default_volume[0] == '\0') + return NULL; + + /* + * Verify that the volume still exists. We don't cache this because + * the volume could be dropped after the GUC was set. + */ + if (!OidIsValid(get_foreign_volume_oid(iceberg_default_volume, true))) + return NULL; + + return iceberg_default_volume; +} + +/* + * GetIcebergTableAmOid + * + * Look up the OID of the iceberg table access method, which is provided by + * a datalake extension rather than the kernel. Returns InvalidOid if the + * access method is not installed and missing_ok is true. + */ +Oid +GetIcebergTableAmOid(bool missing_ok) +{ + return get_table_am_oid(ICEBERG_TABLE_AM_NAME, missing_ok); +} Review Comment: In the b4704be776f814b46a5a59eb543df80fad32a864 commit description I read " The iceberg table access method is resolved strictly by name (get_table_am_oid) and is expected to be provided by a datalake extension; without it, CREATE ICEBERG TABLE fails up front with a hint. The kernel does not hardcode any extension name or AM OID." In my opinion, it is strange to have queries (like `CREATE ICEBERG TABLE`) in the core which are useless without extensions. What about default iceberg AM which can be replaced (using `USING`) with AM from some extension? Without default AM we cannot test the code properly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
