Skip to content

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

Merged
huaxingao merged 10 commits into
apache:mainfrom
majian1998:fix-parser-fu-4
Nov 6, 2025
Merged

huaxingao merged 10 commits into
apache:mainfrom
majian1998:fix-parser-fu-4

Conversation

@majian1998

Copy link
Copy Markdown
Contributor

Related PR: #13625, #14483

Some follow-up code optimizations. Thanks to @kamcheungting-db for the suggestions.

@github-actions github-actions Bot added the spark label Nov 4, 2025
@manuzhang
manuzhang requested a review from huaxingao November 4, 2025 04:07
}

private static ParserInterface getNextDelegateParser(ParserInterface parser) {
Logger logger = LoggerFactory.getLogger(ExtendedParser.class);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we not declare this as private static final member of the interface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interface fields cannot be private in Java.
Can the code be written like this?

public interface ExtendedParser extends ParserInterface {
  Logger LOG = LoggerFactory.getLogger(ExtendedParser.class);

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

current = next;
current = getNextDelegateParser(current);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this fixing an actual issue? It would be good to have a test for this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This just simplifies the code. The logic stays the same. It’s a follow-up PR, and the original tests already cover this.

@kamcheungting-db kamcheungting-db left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you @majian1998

Comment thread spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/ExtendedParser.java Outdated
@huaxingao
huaxingao merged commit caa42f6 into apache:main Nov 6, 2025
27 checks passed
@huaxingao

Copy link
Copy Markdown
Contributor

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

thomaschow pushed a commit to thomaschow/iceberg that referenced this pull request Jan 19, 2026
…endedParser in parser chains (apache#14497)

* Follow-up: Support recursive delegate unwrapping to find ExtendedParser in parser chains

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

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

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

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

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

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

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

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

* Follow-up: Optimize support recursive delegate unwrapping to find ExtendedParser in parser chains
talatuyarer pushed a commit to talatuyarer/iceberg that referenced this pull request Apr 1, 2026
…endedParser in parser chains (apache#14497)

* Follow-up: Support recursive delegate unwrapping to find ExtendedParser in parser chains

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

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

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

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

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

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

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

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

* Follow-up: Optimize support recursive delegate unwrapping to find ExtendedParser in parser chains
kevinjqliu added a commit that referenced this pull request May 13, 2026
…er in parser chains (#16306)

Backport of #14483 (and follow-up #14497) to spark/v3.4.
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.

7 participants