Refactor read-only iceberg table errors

Signed-off-by: Aykut Bozkurt <[email protected]>

Author: sfc-gh-abozkurt

pg_lake_copy/src/copy/copy.c

@@ -389,8 +389,6 @@ ProcessPgLakeCopyFrom(CopyStmt *copyStmt, ParseState *pstate, Relation relation,
 	if (PgLakeCopyValidityCheckHook)
 		PgLakeCopyValidityCheckHook(relationId);
 
-	ErrorIfReadOnlyIcebergTable(relationId);
-
 	/* check read-only transaction and parallel mode */
 	if (XactReadOnly)
 	{

pg_lake_engine/include/pg_lake/util/rel_utils.h

@@ -34,7 +34,6 @@ extern PGDLLEXPORT PgLakeTableType GetPgLakeTableTypeViaServerName(char *serverN
 extern PGDLLEXPORT bool IsPgLakeForeignTableById(Oid foreignTableId);
 extern PGDLLEXPORT bool IsPgLakeIcebergForeignTableById(Oid foreignTableId);
 extern PGDLLEXPORT bool IsPgLakeServerName(const char *serverName);
-extern PGDLLEXPORT bool IsAnyWritableLakeTable(Oid foreignTableId);
 extern PGDLLEXPORT bool IsPgLakeIcebergServerName(const char *serverName);
 extern PGDLLEXPORT char *GetWritableTableLocation(Oid relationId, char **queryArguments);
 extern PGDLLEXPORT void EnsureTableOwner(Oid relationId);

pg_lake_engine/src/utils/rel_utils.c

@@ -80,22 +80,6 @@ GetPgLakeForeignServerName(Oid foreignTableId)
 }
 
 
-/*
-* IsAnyWritableLakeTable - check if the table is writable.
-*/
-bool
-IsAnyWritableLakeTable(Oid foreignTableId)
-{
-	ForeignTable *foreignTable = GetForeignTable(foreignTableId);
-	List	   *options = foreignTable->options;
-	DefElem    *writableOption = GetOption(options, "writable");
-	PgLakeTableType tableType = GetPgLakeTableType(foreignTableId);
-
-	return tableType == PG_LAKE_ICEBERG_TABLE_TYPE ||
-		(writableOption != NULL ? defGetBoolean(writableOption) : false);
-}
-
-
 /*
 * IsAnyLakeForeignTableById - check if the table is a lake table.
 */

pg_lake_iceberg/include/pg_lake/iceberg/catalog.h

@@ -62,10 +62,7 @@ extern PGDLLEXPORT void UpdateInternalCatalogMetadataLocation(Oid relationId, co
 extern PGDLLEXPORT void UpdateAllInternalIcebergTablesToReadOnly(void);
 extern PGDLLEXPORT const char *GetIcebergDefaultLocationPrefix(void);
 extern PGDLLEXPORT bool IcebergTablesCatalogExists(void);
-extern PGDLLEXPORT void ErrorIfReadOnlyIcebergTable(Oid relationId);
-extern PGDLLEXPORT bool WarnIfReadOnlyIcebergTable(Oid relationId);
-extern PGDLLEXPORT void ErrorIfReadOnlyExternalCatalogIcebergTable(Oid relationId);
-extern PGDLLEXPORT bool IsReadOnlyIcebergTable(Oid relationId);
 extern PGDLLEXPORT bool RelationExistsInTheIcebergCatalog(Oid relationId);
 extern PGDLLEXPORT bool HasCustomLocation(Oid relationId);
 extern PGDLLEXPORT bool IsWritableIcebergTable(Oid relationId);
+extern PGDLLEXPORT bool IsReadOnlyIcebergTable(Oid relationId);

pg_lake_iceberg/src/iceberg/catalog.c

@@ -38,7 +38,6 @@ static char *GetIcebergExternalMetadataLocation(Oid relationId);
 static char *GetIcebergCatalogMetadataLocationInternal(Oid relationId, bool isPrevMetadata, bool forUpdate);
 static char *GetIcebergCatalogColumnInternal(Oid relationId, char *columnName, bool forUpdate, bool errorIfNotFound);
 static void ErrorIfSameTableExistsInExternalCatalog(Oid relationId);
-static bool ReportIfReadOnlyIcebergTable(Oid relationId, int logLevel);
 
 /*
  * InsertExternalIcebergCatalogTable inserts a record into the Iceberg
@@ -436,65 +435,6 @@ GetIcebergCatalogPreviousMetadataLocation(Oid relationId, bool forUpdate)
 	return GetIcebergCatalogMetadataLocationInternal(relationId, true, forUpdate);
 }
 
-/*
-* ErrorIfReadOnlyIcebergTable checks if the iceberg table is read-only and
-* throws an error if it is.
-*/
-void
-ErrorIfReadOnlyIcebergTable(Oid relationId)
-{
-	ReportIfReadOnlyIcebergTable(relationId, ERROR);
-
-	ErrorIfReadOnlyExternalCatalogIcebergTable(relationId);
-}
-
-/*
-* WarnIfReadOnlyIcebergTable checks if the iceberg table is read-only and
-* throws a warning if it is.
-*/
-bool
-WarnIfReadOnlyIcebergTable(Oid relationId)
-{
-	return ReportIfReadOnlyIcebergTable(relationId, WARNING);
-}
-
-/*
-* Similar to ErrorIfReadOnlyExternalCatalogIcebergTable, but for external
-* catalog iceberg tables, namely rest catalog and object catalog tables.
-*/
-void
-ErrorIfReadOnlyExternalCatalogIcebergTable(Oid relationId)
-{
-	IcebergCatalogType icebergCatalogType = GetIcebergCatalogType(relationId);
-
-	if (icebergCatalogType == REST_CATALOG_READ_ONLY ||
-		icebergCatalogType == OBJECT_STORE_READ_ONLY)
-		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						errmsg("modifications on read-only external catalog iceberg tables are not supported")));
-}
-
-
-/*
-* ReportIfReadOnlyIcebergTable checks if the iceberg table is read-only and
-* reports an logLevel if it is.
-*
-* For non-error cases, it returns true if the table is read-only.
-*/
-static bool
-ReportIfReadOnlyIcebergTable(Oid relationId, int logLevel)
-{
-	bool		readOnly = IsReadOnlyIcebergTable(relationId);
-
-	if (readOnly)
-	{
-		ereport(logLevel,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("iceberg table \"%s\" is read-only", get_rel_name(relationId))));
-	}
-
-	return readOnly;
-}
-
 /*
 * GetIcebergCatalogMetadataLocationInternal returns the metadata or previous metadata
 * location for a table in the iceberg catalog table.
@@ -509,36 +449,6 @@ GetIcebergCatalogMetadataLocationInternal(Oid relationId, bool isPrevMetadata, b
 }
 
 
-/*
-* IsReadOnlyIcebergTable checks if the iceberg table is read-only and
-* returns true if it is.
-*/
-bool
-IsReadOnlyIcebergTable(Oid relationId)
-{
-	if (GetPgLakeTableType(relationId) != PG_LAKE_ICEBERG_TABLE_TYPE)
-	{
-		/* read-only feature is only applicable for pg_lake_iceberg tables */
-		return false;
-	}
-
-	bool		forUpdate = false;
-	char	   *columnName = "read_only";
-	bool		errorIfNotFound = false;
-
-
-	char	   *readOnlyValue =
-		GetIcebergCatalogColumnInternal(relationId, columnName, forUpdate, errorIfNotFound);
-
-	if (readOnlyValue != NULL && pg_strcasecmp(readOnlyValue, "t") == 0)
-	{
-		/* let the caller know that this is a read-only table for non-errors */
-		return true;
-	}
-
-	return false;
-}
-
 
 /*
 * RelationExistsInTheIcebergCatalog checks if the relation exists in the iceberg
@@ -808,3 +718,13 @@ IsWritableIcebergTable(Oid relationId)
 
 	return (pg_strcasecmp(readOnlyValue, "f") == 0);
 }
+
+
+/*
+ * IsReadOnlyIcebergTable - check if the iceberg table is read-only.
+ */
+bool
+IsReadOnlyIcebergTable(Oid relationId)
+{
+	return IsIcebergTable(relationId) && !IsWritableIcebergTable(relationId);
+}

pg_lake_table/src/ddl/alter_table.c

@@ -205,6 +205,7 @@ static bool HasPartitionByAdded(AlterTableStmt *alterStmt);
 static bool HasPartitionByDropped(AlterTableStmt *alterStmt);
 static bool HasOnlyCatalogAlterTableOptions(AlterTableStmt *alterStmt);
 static void ErrorIfUnsupportedTableOptionChange(AlterTableStmt *alterStmt, List *allowedOptions);
+static void ErrorIfAlterReadOnlyIcebergTable(Oid relationId);
 
 /*
  * ProcessAlterTable is used in cases where we want to preempt the error
@@ -228,8 +229,7 @@ ProcessAlterTable(ProcessUtilityParams * processUtilityParams, void *arg)
 
 	Oid			relationId = AlterTableLookupRelation(alterStmt, NoLock);
 
-	if (!IsWritablePgLakeTable(relationId) &&
-		!IsPgLakeIcebergForeignTableById(relationId))
+	if (!IsWritablePgLakeTable(relationId) && !IsIcebergTable(relationId))
 	{
 		/* for non-pg_lake tables, error when using SET ACCESS METHOD iceberg */
 		ErrorIfUnsupportedSetAccessMethod(alterStmt);
@@ -272,8 +272,7 @@ ProcessAlterTable(ProcessUtilityParams * processUtilityParams, void *arg)
 
 	}
 
-	/* check whether we are accepting writes for this table */
-	ErrorIfReadOnlyIcebergTable(relationId);
+	ErrorIfAlterReadOnlyIcebergTable(relationId);
 
 	ErrorIfUnsupportedTypeAddedForIcebergTables(alterStmt);
 
@@ -634,13 +633,12 @@ PostProcessRenameWritablePgLakeTable(ProcessUtilityParams * params, void *arg)
 
 	Oid			relationId = get_relname_relid(relationName, namespaceId);
 
-	if (!IsWritablePgLakeTable(relationId) &&
-		!IsPgLakeIcebergForeignTableById(relationId))
+	if (!IsWritablePgLakeTable(relationId) && !IsIcebergTable(relationId))
 	{
 		return;
 	}
 
-	ErrorIfReadOnlyIcebergTable(relationId);
+	ErrorIfAlterReadOnlyIcebergTable(relationId);
 
 	PgLakeTableType tableType = GetPgLakeTableType(relationId);
 
@@ -706,13 +704,12 @@ PostProcessAlterWritablePgLakeTableSchema(ProcessUtilityParams * params, void *a
 
 	Oid			relationId = get_relname_relid(alterSchemaStmt->relation->relname, namespaceId);
 
-	if (!IsWritablePgLakeTable(relationId) &&
-		!IsPgLakeIcebergForeignTableById(relationId))
+	if (!IsWritablePgLakeTable(relationId) && !IsIcebergTable(relationId))
 	{
 		return;
 	}
 
-	ErrorIfReadOnlyIcebergTable(relationId);
+	ErrorIfAlterReadOnlyIcebergTable(relationId);
 
 	PgLakeTableType tableType = GetPgLakeTableType(relationId);
 
@@ -1368,6 +1365,20 @@ ErrorIfUnsupportedTableOptionChange(AlterTableStmt *alterStmt, List *allowedOpti
 }
 
 
+/*
+ * ErrorIfAlterReadOnlyIcebergTable throws an error if an attempt is made to
+ * alter a read-only lake table.
+ */
+static void
+ErrorIfAlterReadOnlyIcebergTable(Oid relationId)
+{
+	if (IsReadOnlyIcebergTable(relationId))
+	{
+		ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("ALTER TABLE is not supported for read-only iceberg tables")));
+	}
+}
+
 
 /*
  * HasPartitionByAdded checks if new partition_by option is set or added.

pg_lake_table/src/ddl/ddl_changes.c

@@ -134,15 +134,14 @@ ApplyDDLCatalogChanges(Oid relationId, List *ddlOperations,
 			}
 
 			/*
-			 * We intentionally do not call ErrorIfReadOnlyIcebergTable here
-			 * because users might want to get rid-of the iceberg table on
-			 * forks. If we ever remove the remote files with DROP TABLE, we
-			 * should call ErrorIfReadOnlyIcebergTable here.
+			 * We intentionally do not throw error here because users might
+			 * want to get rid-of the iceberg table on forks. If we ever
+			 * remove the remote files with DROP TABLE, we should throw error.
 			 *
 			 * Also, for read-only tables, we will not mark the files for
 			 * deletion.
 			 */
-			if (!IsReadOnlyIcebergTable(relationId))
+			if (IsWritableIcebergTable(relationId))
 			{
 				/*
 				 * metadata is not pushed yet if table is created in current

pg_lake_table/src/ddl/vacuum.c

@@ -526,8 +526,6 @@ VacuumLakeTables(ProcessUtilityParams * utilityParams)
 										 get_rel_name(relationId))));
 			}
 
-			ErrorIfReadOnlyExternalCatalogIcebergTable(relationId);
-
 			continue;
 		}
 
@@ -563,8 +561,13 @@ VacuumLakeTables(ProcessUtilityParams * utilityParams)
 	{
 		Oid			relationId = lfirst_oid(relationIdCell);
 
-		if (WarnIfReadOnlyIcebergTable(relationId))
+		/* skip read only lake tables */
+		if (!IsWritablePgLakeTable(relationId) && !IsWritableIcebergTable(relationId))
 		{
+			ereport(WARNING,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("lake table \"%s\" is read-only", get_rel_name(relationId))));
+
 			/* let other tables VACUUMed */
 			continue;
 		}

pg_lake_table/src/fdw/pg_lake_table.c

@@ -515,6 +515,7 @@ static void ErrorIfSystemColumnUsed(Oid relationId, AttrNumber attnum);
 static bool AdjustUniqueRelationIdentifiersViaAlias(Node *node, void *context);
 static bool IsSimpleDelete(ModifyTableState *mtstate, Relation resultRel);
 static void ShutdownPgLakeScan(ForeignScanState *node);
+static void ErrorIfTruncateReadOnlyLakeTable(Oid relationId);
 
 
 /*
@@ -2551,7 +2552,7 @@ postgresIsForeignRelUpdatable(Relation rel)
 	 * By default, all pg_lake foreign tables are assumed not writable. This
 	 * can be overridden by a per-table setting.
 	 */
-	if (!IsAnyWritableLakeTable(relationId))
+	if (!IsWritablePgLakeTable(relationId) && !IsWritableIcebergTable(relationId))
 		return 0;
 
 	int			writeFlags = (1 << CMD_INSERT);
@@ -3360,8 +3361,6 @@ create_foreign_modify(Relation rel,
 	CmdType		operation = mtstate->operation;
 	Oid			relationId = RelationGetRelid(rel);
 
-	ErrorIfReadOnlyIcebergTable(relationId);
-
 	/* extra checks */
 	if (PgLakeModifyValidityCheckHook)
 		PgLakeModifyValidityCheckHook(relationId);
@@ -4412,7 +4411,7 @@ postgresExecForeignTruncate(List *relations,
 		Relation	relation = lfirst(relationCell);
 		Oid			relationId = RelationGetRelid(relation);
 
-		ErrorIfReadOnlyIcebergTable(relationId);
+		ErrorIfTruncateReadOnlyLakeTable(relationId);
 
 		/* extra checks */
 		if (PgLakeModifyValidityCheckHook)
@@ -5789,3 +5788,17 @@ ShutdownPgLakeScan(ForeignScanState *node)
 
 	estate->es_processed += fsstate->skippableRows;
 }
+
+
+/*
+ * ErrorIfTruncateReadOnlyLakeTable
+ *		Raise an error if trying to TRUNCATE a read-only lake table.
+ */
+static void
+ErrorIfTruncateReadOnlyLakeTable(Oid relationId)
+{
+	if (!IsWritablePgLakeTable(relationId) && !IsWritableIcebergTable(relationId))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("TRUNCATE on read-only lake tables is not supported")));
+}

pg_lake_table/src/planner/insert_select.c

@@ -19,6 +19,7 @@
 
 #include "access/table.h"
 #include "pg_lake/extensions/postgis.h"
+#include "pg_lake/iceberg/catalog.h"
 #include "pg_lake/planner/insert_select.h"
 #include "pg_lake/planner/query_pushdown.h"
 #include "pg_lake/util/numeric.h"
@@ -84,10 +85,10 @@ IsPushdownableInsertSelectQuery(Query *query)
 
 	Oid			insertIntoRelid = GetInsertRelidFromInsertSelect(query);
 
-	if (!IsPgLakeIcebergForeignTableById(insertIntoRelid))
+	if (!IsWritablePgLakeTable(insertIntoRelid) && !IsWritableIcebergTable(insertIntoRelid))
 	{
 		ereport(DEBUG4,
-				(errmsg("INSERT..SELECT into non-pg_lake table is not pushdownable")));
+				(errmsg("INSERT..SELECT into read-only pg_lake table is not pushdownable")));
 
 		return false;
 	}