Skip to content

Commit

Permalink
Remove deprecated arguments from Iceberg table_changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@ebyhr
ebyhr committed Dec 2, 2024
1 parent a0e1ea3 commit 878f2bc
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 61 deletions.
37 changes: 2 additions & 35 deletions ...rg/src/main/java/io/trino/plugin/iceberg/functions/tablechanges/TableChangesFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
import io.airlift.log.Logger;
import io.airlift.slice.Slice;
import io.trino.plugin.iceberg.ColumnIdentity;
import io.trino.plugin.iceberg.IcebergColumnHandle;
Expand Down Expand Up @@ -65,14 +64,8 @@
public class TableChangesFunction
extends AbstractConnectorTableFunction
{
private static final Logger log = Logger.get(TableChangesFunction.class);

private static final String FUNCTION_NAME = "table_changes";
@Deprecated
private static final String SCHEMA_VAR_NAME = "SCHEMA";
private static final String SCHEMA_NAME_VAR_NAME = "SCHEMA_NAME";
@Deprecated
private static final String TABLE_VAR_NAME = "TABLE";
private static final String TABLE_NAME_VAR_NAME = "TABLE_NAME";
private static final String START_SNAPSHOT_VAR_NAME = "START_SNAPSHOT_ID";
private static final String END_SNAPSHOT_VAR_NAME = "END_SNAPSHOT_ID";
Expand All @@ -88,14 +81,12 @@ public TableChangesFunction(TrinoCatalogFactory trinoCatalogFactory, TypeManager
FUNCTION_NAME,
ImmutableList.of(
ScalarArgumentSpecification.builder()
.name(SCHEMA_VAR_NAME)
.name(SCHEMA_NAME_VAR_NAME)
.type(VARCHAR)
.defaultValue(null)
.build(),
ScalarArgumentSpecification.builder()
.name(TABLE_VAR_NAME)
.name(TABLE_NAME_VAR_NAME)
.type(VARCHAR)
.defaultValue(null)
.build(),
ScalarArgumentSpecification.builder()
.name(START_SNAPSHOT_VAR_NAME)
Expand All @@ -104,16 +95,6 @@ public TableChangesFunction(TrinoCatalogFactory trinoCatalogFactory, TypeManager
ScalarArgumentSpecification.builder()
.name(END_SNAPSHOT_VAR_NAME)
.type(BIGINT)
.build(),
ScalarArgumentSpecification.builder()
.name(SCHEMA_NAME_VAR_NAME)
.type(VARCHAR)
.defaultValue(null)
.build(),
ScalarArgumentSpecification.builder()
.name(TABLE_NAME_VAR_NAME)
.type(VARCHAR)
.defaultValue(null)
.build()),
GENERIC_TABLE);

Expand Down Expand Up @@ -201,13 +182,6 @@ public TableFunctionAnalysis analyze(ConnectorSession session, ConnectorTransact

private static String getSchemaName(Map<String, Argument> arguments)
{
if (argumentExists(arguments, SCHEMA_VAR_NAME) && argumentExists(arguments, SCHEMA_NAME_VAR_NAME)) {
throw new TrinoException(INVALID_FUNCTION_ARGUMENT, "Cannot use both " + SCHEMA_VAR_NAME + " and " + SCHEMA_NAME_VAR_NAME + " arguments");
}
if (argumentExists(arguments, SCHEMA_VAR_NAME)) {
log.warn("%s argument is deprecated. Use %s instead.", SCHEMA_VAR_NAME, SCHEMA_NAME_VAR_NAME);
return ((Slice) checkNonNull(((ScalarArgument) arguments.get(SCHEMA_VAR_NAME)).getValue())).toStringUtf8();
}
if (argumentExists(arguments, SCHEMA_NAME_VAR_NAME)) {
return ((Slice) checkNonNull(((ScalarArgument) arguments.get(SCHEMA_NAME_VAR_NAME)).getValue())).toStringUtf8();
}
Expand All @@ -216,13 +190,6 @@ private static String getSchemaName(Map<String, Argument> arguments)

private static String getTableName(Map<String, Argument> arguments)
{
if (argumentExists(arguments, TABLE_VAR_NAME) && argumentExists(arguments, TABLE_NAME_VAR_NAME)) {
throw new TrinoException(INVALID_FUNCTION_ARGUMENT, "Cannot use both " + TABLE_VAR_NAME + " and " + TABLE_NAME_VAR_NAME + " arguments");
}
if (argumentExists(arguments, TABLE_VAR_NAME)) {
log.warn("%s argument is deprecated. Use %s instead.", TABLE_VAR_NAME, TABLE_NAME_VAR_NAME);
return ((Slice) checkNonNull(((ScalarArgument) arguments.get(TABLE_VAR_NAME)).getValue())).toStringUtf8();
}
if (argumentExists(arguments, TABLE_NAME_VAR_NAME)) {
return ((Slice) checkNonNull(((ScalarArgument) arguments.get(TABLE_NAME_VAR_NAME)).getValue())).toStringUtf8();
}
Expand Down
17 changes: 0 additions & 17 deletions ...in/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -762,23 +762,6 @@ public void testTableChangesFunction()
.formatted(table.getName(), initialSnapshot, snapshotAfterInsert),
"SELECT nationkey, name, 'insert', %s, '%s', 0 FROM nation".formatted(snapshotAfterInsert, snapshotAfterInsertTime));

// Run with deprecated named arguments
assertQuery(
"SELECT nationkey, name, _change_type, _change_version_id, to_iso8601(_change_timestamp), _change_ordinal " +
"FROM TABLE(system.table_changes(\"SCHEMA\" => CURRENT_SCHEMA, \"TABLE\" => '%s', start_snapshot_id => %s, end_snapshot_id => %s))"
.formatted(table.getName(), initialSnapshot, snapshotAfterInsert),
"SELECT nationkey, name, 'insert', %s, '%s', 0 FROM nation".formatted(snapshotAfterInsert, snapshotAfterInsertTime));
assertQuery(
"SELECT nationkey, name, _change_type, _change_version_id, to_iso8601(_change_timestamp), _change_ordinal " +
"FROM TABLE(system.table_changes(schema_name => CURRENT_SCHEMA, \"TABLE\" => '%s', start_snapshot_id => %s, end_snapshot_id => %s))"
.formatted(table.getName(), initialSnapshot, snapshotAfterInsert),
"SELECT nationkey, name, 'insert', %s, '%s', 0 FROM nation".formatted(snapshotAfterInsert, snapshotAfterInsertTime));
assertQuery(
"SELECT nationkey, name, _change_type, _change_version_id, to_iso8601(_change_timestamp), _change_ordinal " +
"FROM TABLE(system.table_changes(\"SCHEMA\" => CURRENT_SCHEMA, table_name => '%s', start_snapshot_id => %s, end_snapshot_id => %s))"
.formatted(table.getName(), initialSnapshot, snapshotAfterInsert),
"SELECT nationkey, name, 'insert', %s, '%s', 0 FROM nation".formatted(snapshotAfterInsert, snapshotAfterInsertTime));

assertUpdate("DELETE FROM " + table.getName(), 25);
long snapshotAfterDelete = getMostRecentSnapshotId(table.getName());
String snapshotAfterDeleteTime = getSnapshotTime(table.getName(), snapshotAfterDelete).format(instantMillisFormatter);
Expand Down
11 changes: 2 additions & 9 deletions plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7811,19 +7811,12 @@ public void testTableChangesFunctionAfterSchemaChange()
@Test
public void testTableChangesFunctionInvalidArguments()
{
assertQueryFails(
"SELECT * FROM TABLE(system.table_changes(\"SCHEMA\" => CURRENT_SCHEMA, \"SCHEMA_NAME\" => CURRENT_SCHEMA, \"TABLE\" => 'region', START_SNAPSHOT_ID => 1, END_SNAPSHOT_ID => 2))",
"Cannot use both SCHEMA and SCHEMA_NAME arguments");
assertQueryFails(
"SELECT * FROM TABLE(system.table_changes(\"SCHEMA\" => CURRENT_SCHEMA, \"TABLE\" => 'region', \"TABLE_NAME\" => 'region', START_SNAPSHOT_ID => 1, END_SNAPSHOT_ID => 2))",
"Cannot use both TABLE and TABLE_NAME arguments");

assertQueryFails(
"SELECT * FROM TABLE(system.table_changes(start_snapshot_id => 1, end_snapshot_id => 2))",
"SCHEMA_NAME argument not found");
".*: Missing argument: SCHEMA_NAME");
assertQueryFails(
"SELECT * FROM TABLE(system.table_changes(schema_name => 'tpch', start_snapshot_id => 1, end_snapshot_id => 2))",
"TABLE_NAME argument not found");
".* Missing argument: TABLE_NAME");
}

@Test
Expand Down

0 comments on commit 878f2bc

Please sign in to comment.