GH-725: Added ExtensionReader #726

xxlaykxx · 2025-04-23T06:22:27Z

What's Changed

ExtensionReader was added to support reading extension types from a complex vector.
It contains read(ExtensionHolder) method for reading to the holder. And readObject - for reading the value explicitly.

Closes #725.

jbonofre · 2025-04-24T08:17:32Z

Thanks for the PR, let me take a look.

xxlaykxx · 2025-05-05T09:45:22Z

@jbonofre @lidavidm could someone plz take a look?

jbonofre · 2025-05-05T09:53:07Z

@xxlaykxx sure. I will.

xxlaykxx · 2025-06-02T15:18:28Z

still need review, plz

lidavidm · 2025-06-04T06:22:14Z

vector/src/main/codegen/templates/AbstractFieldReader.java

  </#list></#list>
+
+  public void read(ExtensionHolder holder) {
+    fail("Extension");


It might be clearer to use the signature as the error message (like other methods do below)?

lidavidm · 2025-06-04T06:23:55Z

vector/src/test/java/org/apache/arrow/vector/complex/impl/UuidReaderImpl.java

+  public void read(ExtensionHolder holder) {
+    UuidHolder uuidHolder = (UuidHolder) holder;
+    vector.get(idx(), uuidHolder);
+  }


Why don't we need to override the other methods in AbstractFieldReader?

added annotation

read(int arrayIndex, ExtensionHolder holder), copyAsValue, copyAsField still aren't implemented?

added read, copyAsValue. copyAsField can't find any usage for other classes where this impl exists.

lidavidm · 2025-06-04T06:24:10Z

vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java

+        FieldReader uuidReader = rootReader.reader("uuid1");
+        uuidReader.setPosition(0);
+        UuidHolder uuidHolder = new UuidHolder();
+        uuidReader.read(uuidHolder);


Similarly we test read but not the other methods?

read(int arrayIndex, ExtensionHolder holder), copyAsValue, copyAsField aren't tested?

Added tests with read and copyAsValue usage. For copyAsField, there are no examples of how it should be used, so not sure what is the right test case for it.

lidavidm

Sorry for the delay, LGTM

lidavidm · 2025-08-05T02:03:43Z

argh...I forgot to reapprove workflows...let me fix thigns

ExtensionReader was added to support reading extension types from a complex vector. It contains **read(ExtensionHolder)** method for reading to the holder. And **readObject** - for reading the value explicitly. Closes apache#725.

Bumps [org.apache.commons:commons-text](https://github.com/apache/commons-text) from 1.13.1 to 1.15.0. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/apache/commons-text/blob/master/RELEASE-NOTES.txt">org.apache.commons:commons-text's changelog</a>.</em></p> <blockquote> <h2>Apache Commons Text 1.15.0 Release Notes</h2> <p>The Apache Commons Text team is pleased to announce the release of Apache Commons Text 1.15.0.</p> <p>Apache Commons Text is a set of utility functions and reusable components for processing and manipulating text in a Java environment.</p> <p>Release 1.15.0. This is a feature and maintenance release. Java 8 or later is required.</p> <h2>New features</h2> <ul> <li> <pre><code> Add experimental CycloneDX VEX file [#683](apache/commons-text#683). Thanks to Piotr P. Karwasz, Gary Gregory. </code></pre> </li> <li>TEXT-235: Add Damerau-Levenshtein distance <a href="https://redirect.github.com/apache/commons-text/issues/687">#687</a>. Thanks to LorgeN, Gary Gregory.</li> <li> <pre><code> Add unit tests to increase coverage [#719](apache/commons-text#719). Thanks to Michael Hausegger, Gary Gregory. </code></pre> </li> <li> <pre><code> Add new test for CharSequenceTranslator#with() [#725](apache/commons-text#725). Thanks to Michael Hausegger, Gary Gregory. </code></pre> </li> <li> <pre><code> Add tests and assertions to org.apache.commons.text.similarity to get to 100% code coverage [#727](apache/commons-text#727), [#728](apache/commons-text#728). Thanks to Michael Hausegger. </code></pre> </li> </ul> <h2>Fixed Bugs</h2> <ul> <li> <pre><code> Fix exception message typo in XmlStringLookup.XmlStringLookup(Map, Path...). Thanks to Gary Gregory. </code></pre> </li> <li>TEXT-236: Inserting at the end of a TextStringBuilder throws a StringIndexOutOfBoundsException. Thanks to Pierre Post, Sumit Bera, Alex Herbert, Gary Gregory.</li> <li> <pre><code> Fix TextStringBuilderTest.testAppendToCharBuffer() to use proper argument type [#724](apache/commons-text#724). Thanks to Michael Hausegger. </code></pre> </li> <li> <pre><code> Fix Apache RAT plugin console warnings. Thanks to Gary Gregory. </code></pre> </li> <li> <pre><code> Fix site XML to use version 2.0.0 XML schema. Thanks to Gary Gregory. </code></pre> </li> <li> <pre><code> Removed unreachable threshold verification code in src/main/java/org/apache/commons/text/similarity [#730](apache/commons-text#730). Thanks to Michael Hausegger. </code></pre> </li> <li> <pre><code> Enable secure processing for the XML parser in XmlStringLookup in case the underlying JAXP implementation doesn't [#729](apache/commons-text#729). Thanks to 김민재 (minjas0507), Gary Gregory, Piotr Karwasz. </code></pre> </li> </ul> <h2>Changes</h2> <ul> <li> <pre><code> Bump org.apache.commons:commons-parent from 85 to 93 [#704](apache/commons-text#704), [#723](apache/commons-text#723), [#726](apache/commons-text#726). Thanks to Gary Gregory. </code></pre> </li> <li> <pre><code> Bump commons.bytebuddy.version from 1.17.6 to 1.18.2 [#696](apache/commons-text#696), [#722](apache/commons-text#722). Thanks to Gary Gregory. </code></pre> </li> <li> <pre><code> Bump graalvm.version from 24.2.2 to 25.0.1 [#703](apache/commons-text#703), [#716](apache/commons-text#716). Thanks to Gary Gregory, Dependabot. </code></pre> </li> <li> <pre><code> Bump org.apache.commons:commons-lang3 from 3.18.0 to 3.20.0. Thanks to Gary Gregory. </code></pre> </li> <li> <pre><code> Bump commons-io:commons-io from 2.20.0 to 2.21.0. Thanks to Gary Gregory. </code></pre> </li> </ul> <p>Historical list of changes: <a href="https://commons.apache.org/proper/commons-text/changes.html">https://commons.apache.org/proper/commons-text/changes.html</a></p> <p>For complete information on Apache Commons Text, including instructions on how to submit bug reports, patches, or suggestions for improvement, see the Apache Commons Text website:</p> <p><a href="https://commons.apache.org/proper/commons-text">https://commons.apache.org/proper/commons-text</a></p> <p>Download page: <a href="https://commons.apache.org/proper/commons-text/download_text.cgi">https://commons.apache.org/proper/commons-text/download_text.cgi</a></p>  </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/apache/commons-text/commit/04e937470d3679cc163df85d82d5b6d2e3e71128"><code>04e9374</code></a> Prepare for the release candidate 1.15.0 RC1</li> <li><a href="https://github.com/apache/commons-text/commit/502c4c41be5671681b58a9b50297f99737e8ea93"><code>502c4c4</code></a> Prepare for the next release candidate</li> <li><a href="https://github.com/apache/commons-text/commit/c6e17ec24cc8374eb12676b717bf797f41b6e539"><code>c6e17ec</code></a> Use direct access</li> <li><a href="https://github.com/apache/commons-text/commit/58e1e125daaa0aebf8c5ffaa82af48821a1ccf2d"><code>58e1e12</code></a> Simplify XML FSP (<a href="https://redirect.github.com/apache/commons-text/issues/731">#731</a>)</li> <li><a href="https://github.com/apache/commons-text/commit/b5052c97e84e1c174ec8bfbbb749e33f22917a07"><code>b5052c9</code></a> Bump actions/setup-java from 5.0.0 to 5.1.0</li> <li><a href="https://github.com/apache/commons-text/commit/2e2d4bc90f1b3274e7943ac27d037d47c0cc098d"><code>2e2d4bc</code></a> Revert "Bump actions/setup-java from 5.0.0 to 5.1.0"</li> <li><a href="https://github.com/apache/commons-text/commit/b0ddbd17bbeee12ad33b8a61c60b4edbe6c85838"><code>b0ddbd1</code></a> Bump actions/setup-java from 5.0.0 to 5.1.0</li> <li><a href="https://github.com/apache/commons-text/commit/1c2d3821e67e08342b8cef4d4445c30b4a22daca"><code>1c2d382</code></a> Add tests with external DTD</li> <li><a href="https://github.com/apache/commons-text/commit/ed3df4b25cd5301921a6523ae7db2411f4a84d98"><code>ed3df4b</code></a> Internal clean up</li> <li><a href="https://github.com/apache/commons-text/commit/bb508f304a8835ac2319af1d872b2f1a9ff6f81d"><code>bb508f3</code></a> Bump actions/checkout from 6.0.0 to 6.0.1</li> <li>Additional commits viewable in <a href="https://github.com/apache/commons-text/compare/rel/commons-text-1.13.1...rel/commons-text-1.15.0">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=org.apache.commons:commons-text&package-manager=maven&previous-version=1.13.1&new-version=1.15.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

apacheGH-725: Added ExtensionReader

e1f3892

This comment has been minimized.

Sign in to view

lidavidm added the enhancement PRs that add or improve features. label Apr 23, 2025

xxlaykxx marked this pull request as ready for review April 23, 2025 16:24

xxlaykxx requested review from jbonofre, laurentgo, lidavidm and wgtmac as code owners April 23, 2025 16:24

github-actions bot added this to the 18.3.0 milestone Apr 23, 2025

fixed code style AbstractFieldReader.java

928519b

jbonofre modified the milestones: 18.3.0, 18.4.0 May 8, 2025

lidavidm reviewed Jun 4, 2025

View reviewed changes

apacheGH-725: updated tests

629d9af

xxlaykxx requested a review from lidavidm June 11, 2025 08:58

xxlaykxx added 4 commits June 24, 2025 18:12

apacheGH-725: updated tests

84bf650

apacheGH-725: updated tests

f9c3363

apacheGH-725: fixed UnionListWriter

40089aa

apacheGH-725: test for list reader for extension type

3fe5c93

lidavidm approved these changes Aug 5, 2025

View reviewed changes

lidavidm merged commit e6da71e into apache:main Aug 5, 2025

GH-725: Added ExtensionReader #726

GH-725: Added ExtensionReader #726

Uh oh!

Conversation

xxlaykxx commented Apr 23, 2025

What's Changed

Uh oh!

This comment has been minimized.

jbonofre commented Apr 24, 2025

Uh oh!

xxlaykxx commented May 5, 2025

Uh oh!

jbonofre commented May 5, 2025

Uh oh!

xxlaykxx commented Jun 2, 2025

Uh oh!

lidavidm Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

lidavidm Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

xxlaykxx Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

lidavidm Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

xxlaykxx Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

lidavidm Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

lidavidm Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

xxlaykxx Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

lidavidm commented Aug 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xxlaykxx Jun 24, 2025 •

edited

Loading