amogh-jahagirdar commented on code in PR #13106:
URL: https://github.com/apache/iceberg/pull/13106#discussion_r2183832620
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestWriteAborts.java:
##########
@@ -121,7 +121,7 @@ public void testBatchAppend() throws IOException {
"Should be no orphan data files",
ImmutableList.of(),
sql(
- "CALL %s.system.remove_orphan_files(table => '%s', older_than =>
%dL, location => '%s')",
+ "CALL %s.system.remove_orphan_files(table => '%s', older_than =>
CAST(%dL AS TIMESTAMP), location => '%s')",
Review Comment:
Yeah it's unfortunate but it is a major version upgrade so I think it's OK.
I also don't think the iceberg should get too involved in trying to work around
spark behavior especially for a major version upgrade.
I do think we should call these out in our procedure docs so that way we can
make it easy for folks to figure out any gotcha's on upgrading
##########
spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCherrypickSnapshotProcedure.java:
##########
@@ -168,22 +168,26 @@ public void testInvalidCherrypickSnapshotCases() {
assertThatThrownBy(
() -> sql("CALL %s.system.cherrypick_snapshot('n', table => 't',
1L)", catalogName))
.isInstanceOf(AnalysisException.class)
- .hasMessage("Named and positional arguments cannot be mixed");
+ .hasMessage(
+ "[UNEXPECTED_POSITIONAL_ARGUMENT] Cannot invoke routine
`cherrypick_snapshot` because it contains positional argument(s) following the
named argument assigned to `table`; please rearrange them so the positional
arguments come first and then retry the query again. SQLSTATE: 4274K");
assertThatThrownBy(() -> sql("CALL %s.custom.cherrypick_snapshot('n', 't',
1L)", catalogName))
.isInstanceOf(AnalysisException.class)
- .hasMessage("Catalog %s does not support procedures.", catalogName);
+ .hasMessage(
+ "[FAILED_TO_LOAD_ROUTINE] Failed to load routine
`%s`.`custom`.`cherrypick_snapshot`. SQLSTATE: 38000",
+ catalogName);
assertThatThrownBy(() -> sql("CALL %s.system.cherrypick_snapshot('t')",
catalogName))
.isInstanceOf(AnalysisException.class)
- .hasMessage("Missing required parameters: [snapshot_id]");
+ .hasMessage(
+ "[REQUIRED_PARAMETER_NOT_FOUND] Cannot invoke routine
`cherrypick_snapshot` because the parameter named `snapshot_id` is required,
but the routine call did not supply a value. Please update the routine call to
supply an argument value (either positionally at index 0 or by name) and retry
the query again. SQLSTATE: 4274K");
assertThatThrownBy(() -> sql("CALL %s.system.cherrypick_snapshot('', 1L)",
catalogName))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot handle an empty identifier for argument table");
assertThatThrownBy(() -> sql("CALL %s.system.cherrypick_snapshot('t',
2.2)", catalogName))
- .isInstanceOf(AnalysisException.class)
- .hasMessageStartingWith("Wrong arg type for snapshot_id: cannot cast");
+ .isInstanceOf(RuntimeException.class)
Review Comment:
+1 this test doesn't make sense to me? are we trying to test calling a
procedure on a table that doesn't exist or the table exists and we want to fail
on the cast of the invalid snapshot ID
--
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]
