Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains by majian1998 · Pull Request #13625 · apache/iceberg

majian1998 · 2025-07-22T07:56:29Z

Spark SQL employs a chain of parsers to convert SQL text into logical plans for execution. Many extensions (such as Iceberg, Paimon, etc.) implement their own SQL parsers by wrapping the underlying parser with a delegate. This design allows each extension to support custom syntax, while passing unhandled SQL to the next parser in the chain.

Previously, the Spark/Iceberg logic only checked the outer-most parser instance to determine if it was an ExtendedParser. However, in environments where multiple extensions are stacked (for example, when the Paimon parser delegates to the Iceberg parser), this check fails because the top-level parser is no longer an instance of ExtendedParser. Consequently, features that rely on Iceberg's parser capabilities (such as the custom parseSortOrder logic) would not function correctly in these scenarios.

This PR improves the detection logic by recursively unwrapping delegate parsers until an ExtendedParser instance is found. This change ensures compatibility across multiple parser extensions and improves robustness when integrating with complex Spark SQL extension chains.

Related issue: #8004

I believe this change addresses the above issue.

majian1998 · 2025-07-22T09:23:18Z

Hi @ajantha-bhat , could you please take a look or help review this PR when convenient? Thank you!

majian1998 · 2025-07-28T06:19:44Z

@amogh-jahagirdar @manuzhang Hi! Could you please help review this PR when convenient? Thank you!

manuzhang · 2025-07-28T07:59:30Z

@majian1998 Please target Spark 4.0 first and add a UT if possible.

majian1998 · 2025-07-28T11:45:58Z

@majian1998 Please target Spark 4.0 first and add a UT if possible.

Thanks for your suggestion! I've updated to target Spark 4.0 and added a UT.

majian1998 · 2025-07-30T12:55:31Z

@majian1998 Please target Spark 4.0 first and add a UT if possible.

Could you please review again? Thanks! @manuzhang

github-actions · 2025-08-30T00:16:09Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

github-actions · 2025-09-07T00:17:58Z

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

wForget · 2025-09-08T03:03:37Z

+
+  static Object getDelegate(Object parser) {
+    try {
+      for (String methodName : new String[] {"delegate", "getDelegate"}) {


Should we use getDeclaredFields to iterate over all fields instead of assuming it's always delegate?

Thanks for the suggestion! I've updated the code to iterate over all fields

wForget · 2025-09-08T03:11:44Z

Paimon and Kyuubi integration with iceberg may encounter this issue:

https://github.com/apache/paimon/blob/c937b53d3bdf961993ab53ebe2108d55d8148a4d/paimon-spark/paimon-spark4-common/src/main/scala/org/apache/paimon/spark/catalyst/parser/extensions/PaimonSpark4SqlExtensionsParser.scala#L25-L29

https://github.com/apache/kyuubi/blob/c04dc4fe21d214970d250f816f727cc9bc3a3521/extensions/spark/kyuubi-extension-spark-4-0/src/main/scala/org/apache/kyuubi/sql/KyuubiSparkSQLParser.scala#L124-L128

…er in parser chains

majian1998 · 2025-09-08T03:54:08Z

Paimon and Kyuubi integration with iceberg may encounter this issue:

Yes, I also encountered this issue when integrating with Paimon. This PR should fix the problem.

…er in parser chains

majian1998 · 2025-09-10T07:24:04Z

It looks like this is a common issue with mixed formats. Could you help me continue reviewing this PR? @manuzhang

manuzhang · 2025-09-10T15:27:10Z

@huaxingao could you please help review this PR?

huaxingao · 2025-09-12T00:35:04Z

This approach looks reasonable to me. Also cc @RussellSpitzer @amogh-jahagirdar

…er in parser chains

huaxingao · 2025-10-27T20:29:49Z

@majian1998 can we also add a test where the delegate lives in a superclass?

…er in parser chains

majian1998 · 2025-10-28T02:32:29Z

@majian1998 can we also add a test where the delegate lives in a superclass?

Of course! I’ve added a test to cover superClass field lookup.

…er in parser chains

huaxingao

LGTM

majian1998 · 2025-10-30T05:40:50Z

cc @manuzhang

…er in parser chains

manuzhang · 2025-10-30T12:38:49Z

+  }
+
+  private static class WrapperParser extends AbstractSqlParser {
+    private final ParserInterface delegate;


Can we have more than one levels of superclasses and also more than one fields in each class?

…er in parser chains

huaxingao · 2025-11-01T23:38:00Z

Thanks @majian1998 for the PR! Thanks @manuzhang for the review!

majian1998 · 2025-11-03T02:01:19Z

@manuzhang @huaxingao Thanks! Super excited to contribute to Iceberg for the first time. I’ll apply this fix to Spark 3.4 and 3.5 as well.

…er in parser chains (apache#13625) * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains * Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains

github-actions Bot added the spark label Jul 22, 2025

majian1998 force-pushed the fix-parser branch from 24a84c0 to 6d37ceb Compare July 22, 2025 08:38

majian1998 force-pushed the fix-parser branch 2 times, most recently from 14416e9 to bd8f3dc Compare July 23, 2025 03:14

majian1998 force-pushed the fix-parser branch from bd8f3dc to da35ffc Compare July 28, 2025 11:42

majian1998 changed the title ~~Spark: Support recursive delegate unwrapping to find ExtendedParser in parser chains~~ Spark 4.0: Support recursive delegate unwrapping to find ExtendedParser in parser chains Jul 28, 2025

github-actions Bot added the stale label Aug 30, 2025

github-actions Bot closed this Sep 7, 2025

manuzhang reopened this Sep 7, 2025

github-actions Bot removed the stale label Sep 8, 2025

wForget reviewed Sep 8, 2025

View reviewed changes

majian1998 added 2 commits September 8, 2025 11:24

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

33575fe

…er in parser chains

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

569fc6e

…er in parser chains

majian1998 added 2 commits September 8, 2025 13:35

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

373cba4

…er in parser chains

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

9a49797

…er in parser chains

majian1998 force-pushed the fix-parser branch from 0af9301 to 9a49797 Compare September 8, 2025 05:36

manuzhang requested review from aokolnychyi and huaxingao September 10, 2025 15:24

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

345f856

…er in parser chains

majian1998 requested a review from manuzhang October 23, 2025 06:03

manuzhang requested a review from huaxingao October 23, 2025 06:27

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

c8287d3

…er in parser chains

huaxingao reviewed Oct 27, 2025

View reviewed changes

Comment thread spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/ExtendedParser.java Outdated

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

cb1ca5b

…er in parser chains

majian1998 requested a review from huaxingao October 27, 2025 07:37

huaxingao reviewed Oct 27, 2025

View reviewed changes

Comment thread spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/TestExtendedParser.java

huaxingao reviewed Oct 27, 2025

View reviewed changes

Comment thread spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/TestExtendedParser.java

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

7389176

…er in parser chains

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

5f919b6

…er in parser chains

majian1998 requested a review from huaxingao October 28, 2025 06:07

huaxingao approved these changes Oct 28, 2025

View reviewed changes

manuzhang reviewed Oct 30, 2025

View reviewed changes

Comment thread spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/ExtendedParser.java Outdated

manuzhang reviewed Oct 30, 2025

View reviewed changes

Comment thread spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/TestExtendedParser.java Outdated

majian1998 added 2 commits October 30, 2025 19:01

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

5e1c203

…er in parser chains

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

c4ed761

…er in parser chains

manuzhang reviewed Oct 30, 2025

View reviewed changes

Spark 4.0: Support recursive delegate unwrapping to find ExtendedPars…

dbe7838

…er in parser chains

manuzhang approved these changes Nov 1, 2025

View reviewed changes

huaxingao merged commit aa14aae into apache:main Nov 1, 2025
27 checks passed

This was referenced Nov 3, 2025

Spark 3.5: Support recursive delegate unwrapping to find ExtendedParser in parser chains #14483

Merged

Follow-up: Optimize support recursive delegate unwrapping to find ExtendedParser in parser chains #14497

Merged

Conversation

majian1998 commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

majian1998 commented Jul 22, 2025

Uh oh!

majian1998 commented Jul 28, 2025

Uh oh!

manuzhang commented Jul 28, 2025

Uh oh!

majian1998 commented Jul 28, 2025

Uh oh!

majian1998 commented Jul 30, 2025

Uh oh!

github-actions Bot commented Aug 30, 2025

Uh oh!

github-actions Bot commented Sep 7, 2025

Uh oh!

wForget Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

majian1998 Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

wForget commented Sep 8, 2025

Uh oh!

majian1998 commented Sep 8, 2025

Uh oh!

majian1998 commented Sep 10, 2025

Uh oh!

manuzhang commented Sep 10, 2025

Uh oh!

huaxingao commented Sep 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

huaxingao commented Oct 27, 2025

Uh oh!

majian1998 commented Oct 28, 2025

Uh oh!

huaxingao left a comment

Choose a reason for hiding this comment

Uh oh!

majian1998 commented Oct 30, 2025

Uh oh!

Uh oh!

Uh oh!

manuzhang Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huaxingao commented Nov 1, 2025

Uh oh!

majian1998 commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

majian1998 commented Jul 22, 2025 •

edited

Loading