Skip to content

Commit c123713

Browse files
DDL testts & errors, part-1
1 parent 0ed92c6 commit c123713

File tree

3 files changed

+383
-5
lines changed

3 files changed

+383
-5
lines changed

pg_lake_table/src/ddl/alter_table.c

Lines changed: 110 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "postgres.h"
1919
#include "miscadmin.h"
2020

21+
#include "access/heapam.h"
2122
#include "access/table.h"
2223
#include "access/tableam.h"
2324
#include "catalog/heap.h"
@@ -37,6 +38,7 @@
3738
#include "utils/rel.h"
3839
#include "utils/typcache.h"
3940
#include "utils/inval.h"
41+
#include "utils/fmgroids.h"
4042

4143
#include "pg_lake/access_method/access_method.h"
4244
#include "pg_lake/data_file/data_files.h"
@@ -112,6 +114,8 @@ typedef struct PgLakeDDL
112114
static bool Allowed(Node *arg);
113115
static bool Disallowed(Node *arg);
114116
static bool DisallowedAddColumnWithUnsupportedConstraints(Node *arg);
117+
static bool DisallowedForWritableRestRenameTable(Node *arg);
118+
static bool DisallowedForWritableRestSetSchema(Node *arg);
115119

116120
PgLakeAlterTableHookType PgLakeAlterTableHook = NULL;
117121
PgLakeAlterTableRenameColumnHookType PgLakeAlterTableRenameColumnHook = NULL;
@@ -171,10 +175,10 @@ static const PgLakeDDL PgLakeDDLs[] = {
171175
ALTER_TABLE_DDL(AT_DisableTrigAll, Allowed, Allowed),
172176
ALTER_TABLE_DDL(AT_EnableTrigUser, Allowed, Allowed),
173177
ALTER_TABLE_DDL(AT_DisableTrigUser, Allowed, Allowed),
174-
ALTER_SCHEMA_DDL(OBJECT_TABLE, Allowed, Allowed),
175-
ALTER_SCHEMA_DDL(OBJECT_FOREIGN_TABLE, Allowed, Allowed),
176-
RENAME_TABLE_DDL(OBJECT_TABLE, Allowed, Allowed),
177-
RENAME_TABLE_DDL(OBJECT_FOREIGN_TABLE, Allowed, Allowed),
178+
ALTER_SCHEMA_DDL(OBJECT_TABLE, DisallowedForWritableRestSetSchema, Allowed),
179+
ALTER_SCHEMA_DDL(OBJECT_FOREIGN_TABLE, DisallowedForWritableRestRenameTable, Allowed),
180+
RENAME_TABLE_DDL(OBJECT_TABLE, DisallowedForWritableRestRenameTable, Allowed),
181+
RENAME_TABLE_DDL(OBJECT_FOREIGN_TABLE, DisallowedForWritableRestRenameTable, Allowed),
178182
};
179183

180184
#define N_PG_LAKE_DDLS (sizeof(PgLakeDDLs) / sizeof(PgLakeDDLs[0]))
@@ -205,6 +209,8 @@ static bool HasPartitionByAdded(AlterTableStmt *alterStmt);
205209
static bool HasPartitionByDropped(AlterTableStmt *alterStmt);
206210
static bool HasOnlyCatalogAlterTableOptions(AlterTableStmt *alterStmt);
207211
static void ErrorIfUnsupportedTableOptionChange(AlterTableStmt *alterStmt, List *allowedOptions);
212+
static void ErrorIfAnyRestCatalogTablesInSchema(const char *schemaName);
213+
208214

209215
/*
210216
* ProcessAlterTable is used in cases where we want to preempt the error
@@ -611,6 +617,13 @@ PostProcessRenameWritablePgLakeTable(ProcessUtilityParams * params, void *arg)
611617

612618
RenameStmt *renameStmt = (RenameStmt *) plannedStmt->utilityStmt;
613619

620+
if (renameStmt->renameType == OBJECT_SCHEMA)
621+
{
622+
/* we are in post-process, use new name */
623+
ErrorIfAnyRestCatalogTablesInSchema(renameStmt->newname);
624+
return;
625+
}
626+
614627
if (renameStmt->renameType != OBJECT_COLUMN &&
615628
renameStmt->renameType != OBJECT_ATTRIBUTE &&
616629
renameStmt->renameType != OBJECT_TABLE &&
@@ -1027,6 +1040,58 @@ Disallowed(Node *arg)
10271040
}
10281041

10291042

1043+
/*
1044+
* We have not yet implemented table renames in REST catalog.
1045+
*/
1046+
static bool
1047+
DisallowedForWritableRestRenameTable(Node *arg)
1048+
{
1049+
RenameStmt *renameStmt = (RenameStmt *) arg;
1050+
1051+
/* only disallow RENAME TABLE/FOREIGN TABLE for writable rest catalog */
1052+
if (renameStmt->renameType == OBJECT_TABLE ||
1053+
renameStmt->renameType == OBJECT_FOREIGN_TABLE)
1054+
{
1055+
1056+
Oid namespaceId =
1057+
RangeVarGetAndCheckCreationNamespace(renameStmt->relation, NoLock, NULL);
1058+
1059+
/* we are in the post-process, so should use new name */
1060+
const char *relationName = renameStmt->newname;
1061+
1062+
Oid relationId = get_relname_relid(relationName, namespaceId);
1063+
IcebergCatalogType icebergCatalogType = GetIcebergCatalogType(relationId);
1064+
1065+
return (icebergCatalogType != REST_CATALOG_READ_WRITE);
1066+
}
1067+
1068+
return false;
1069+
}
1070+
1071+
/*
1072+
* We have not yet implemented schema changes in REST catalog.
1073+
*/
1074+
static bool
1075+
DisallowedForWritableRestSetSchema(Node *arg)
1076+
{
1077+
AlterObjectSchemaStmt *alterSchemaStmt = (AlterObjectSchemaStmt *) arg;
1078+
1079+
Oid namespaceId = get_namespace_oid(alterSchemaStmt->newschema, alterSchemaStmt->missing_ok);
1080+
1081+
/* if not validOid, let Postgres handle */
1082+
if (!OidIsValid(namespaceId))
1083+
{
1084+
return false;
1085+
}
1086+
1087+
Oid relationId = get_relname_relid(alterSchemaStmt->relation->relname, namespaceId);
1088+
1089+
IcebergCatalogType icebergCatalogType = GetIcebergCatalogType(relationId);
1090+
1091+
return (icebergCatalogType != REST_CATALOG_READ_WRITE);
1092+
}
1093+
1094+
10301095
/*
10311096
* ErrorIfUnsupportedSetAccessMethod checks ALTER TABLE on non-pg_lake
10321097
* tables for possible issues involving Iceberg.
@@ -1099,6 +1164,8 @@ DisallowedAddColumnWithUnsupportedConstraints(Node *arg)
10991164
return true;
11001165
}
11011166

1167+
1168+
11021169
/*
11031170
* AlterTableAddColumnHasMutableDefault returns true if ALTER TABLE .. ADD COLUMN
11041171
* has a mutable default value. e.g. ALTER TABLE .. ADD COLUMN .. DEFAULT now().
@@ -1397,3 +1464,42 @@ HasPartitionByDropped(AlterTableStmt *alterStmt)
13971464

13981465
return partitionByDropped;
13991466
}
1467+
1468+
1469+
static void
1470+
ErrorIfAnyRestCatalogTablesInSchema(const char *schemaName)
1471+
{
1472+
Oid namespaceId = get_namespace_oid(schemaName, false);
1473+
1474+
Relation classRel = table_open(RelationRelationId, AccessShareLock);
1475+
HeapTuple tuple;
1476+
1477+
ScanKeyData key[1];
1478+
1479+
ScanKeyInit(&key[0],
1480+
Anum_pg_class_relnamespace,
1481+
BTEqualStrategyNumber, F_OIDEQ,
1482+
ObjectIdGetDatum(namespaceId));
1483+
1484+
/* get all the relations present in the specified schema */
1485+
TableScanDesc scan = table_beginscan_catalog(classRel, 1, key);
1486+
1487+
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
1488+
{
1489+
Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
1490+
Oid relid = relForm->oid;
1491+
1492+
IcebergCatalogType icebergCatalogType = GetIcebergCatalogType(relid);
1493+
1494+
if (icebergCatalogType == REST_CATALOG_READ_WRITE)
1495+
{
1496+
ereport(ERROR,
1497+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1498+
errmsg("cannot rename schema \"%s\" because it contains "
1499+
"an iceberg table with rest catalog \"%s\"", schemaName, get_rel_name(relid))));
1500+
}
1501+
}
1502+
1503+
table_endscan(scan);
1504+
table_close(classRel, AccessShareLock);
1505+
}

pg_lake_table/tests/pytests/test_modify_iceberg_rest_table.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"manifest_min_count_to_merge, target_manifest_size_kb, max_snapshot_age_params ",
1212
manifest_snapshot_settings,
1313
)
14-
def test_writable_iceberg_table(
14+
def test_writable_rest_iceberg_table(
1515
installcheck,
1616
install_iceberg_to_duckdb,
1717
spark_session,

0 commit comments

Comments
 (0)