Copilot commented on code in PR #10237:
URL: https://github.com/apache/gravitino/pull/10237#discussion_r2888256978
##########
spark-connector/v3.5/spark/src/main/java/org/apache/gravitino/spark/connector/jdbc/GravitinoJdbcCatalogSpark35.java:
##########
@@ -36,4 +41,10 @@ protected SparkTableChangeConverter
getSparkTableChangeConverter(
SparkTypeConverter sparkTypeConverter) {
return new SparkTableChangeConverter34(sparkTypeConverter);
}
+
+ @Override
+ public Table loadTable(Identifier ident, Set<TableWritePrivilege>
writePrivileges)
+ throws NoSuchTableException {
Review Comment:
`loadTable(Identifier, Set<TableWritePrivilege>)` is only overridden in
`GravitinoJdbcCatalogSpark35`. In the Spark 3.5 module there are JDBC-derived
catalogs (e.g. PostgreSQL) that extend `GravitinoJdbcCatalog` directly and
therefore won’t enforce `MODIFY_TABLE` on write planning, potentially allowing
INSERT/other writes with only LOAD/SELECT privileges. Consider refactoring so
all Spark 3.5 JDBC catalogs share this override (e.g., have JDBC variants
extend `GravitinoJdbcCatalogSpark35` or add the override to a common Spark-3.5
JDBC base).
```suggestion
throws NoSuchTableException {
if (writePrivileges == null
|| !writePrivileges.contains(TableWritePrivilege.MODIFY_TABLE)) {
throw new IllegalArgumentException(
"Write operations require MODIFY_TABLE privilege for table: " +
ident);
}
```
--
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]
