Skip to content

Conversation

@slfan1989
Copy link
Contributor

@slfan1989 slfan1989 commented Sep 25, 2025

Backport #13913

Compared to Spark 4.0, Spark 3.5 is missing PR #13106, which cannot be directly backported. Therefore, some code modifications are required.

@github-actions github-actions bot added the spark label Sep 25, 2025
@slfan1989 slfan1989 changed the title Spark-3.5: Backport: Refactor Spark procedures to consistently use ProcedureInput for parameter handling. Spark 3.5: Backport: Refactor Spark procedures to consistently use ProcedureInput for parameter handling. Sep 25, 2025
Comment on lines +62 to +67
return ProcedureParameter.required(name, dataType);
}

protected static ProcedureParameter optionalInParameter(String name, DataType dataType) {
return ProcedureParameter.optional(name, dataType);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason why we didn't have this in 4.0 ? are we missing backport of some commit to 3.5
seems like not - 30ee7e8

can you please double confirm ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for reviewing the code! This is not a clean backport, since there is a PR (#13106) in the Spark 4.0 branch that cannot be directly backported to Spark 3.4/3.5. Because Spark 3.5 lacks some features compared to Spark 4.0, I made certain adjustments based on the current Spark 3.5 code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Spark 3.5, we use org.apache.spark.sql.connector.iceberg.catalog.ProcedureParameter for parameter parsing, whereas in the Spark 4 branch, org.apache.spark.sql.connector.catalog.procedures.ProcedureParameter is used to achieve the same functionality. Personally, I believe the difference in functionality between the two is not significant.

@nastra nastra merged commit fbb7136 into apache:main Sep 25, 2025
27 checks passed
@slfan1989
Copy link
Contributor Author

@singhpk234 @nastra @huaxingao Thank you very much for helping to review the code!

gabeiglio pushed a commit to gabeiglio/iceberg that referenced this pull request Oct 1, 2025
adawrapub pushed a commit to adawrapub/iceberg that referenced this pull request Oct 16, 2025
cccs-nik pushed a commit to CybercentreCanada/iceberg that referenced this pull request Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants