Skip to content

Conversation

@ffacs
Copy link
Contributor

@ffacs ffacs commented Sep 22, 2024

What changes were proposed in this pull request?

Support Geometry and Geography types for java side

Why are the changes needed?

Add support for Geometry and Geography types

How was this patch tested?

UT passed

Was this patch authored or co-authored using generative AI tooling?

YES

@dongjoon-hyun dongjoon-hyun marked this pull request as draft September 22, 2024 21:31
@ffacs
Copy link
Contributor Author

ffacs commented Sep 23, 2024

cc @wgtmac @dongjoon-hyun

@wgtmac
Copy link
Member

wgtmac commented Sep 24, 2024

Thanks for implementing this! I will hold my review until the spec has been finalized.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Sep 25, 2024

Thank you, @ffacs .

To @wgtmac , do you mean the ORC format PR? Or, Iceberg document? Otherwise, could you provide some pointers to track this part, the spec has been finalized?

Thanks for implementing this! I will hold my review until the spec has been finalized.

java/pom.xml Outdated

<mockito.version>5.10.0</mockito.version>
<orc-format.version>1.0.0</orc-format.version>
<orc-format.version>1.0.0-SNAPSHOT</orc-format.version>
Copy link
Member

Choose a reason for hiding this comment

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

? AFAIK, we didn't change anything relevant until now, do we need -SNAPSHOT?

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 for the review. The spec of geometry type haven't been merged into orc format yet, so I created a private package for developing here. Maybe it's a little bit early for code reviewing, let's waiting for the spec finished first.

java/pom.xml Outdated
<surefire.version>3.0.0-M5</surefire.version>
<test.tmp.dir>${project.build.directory}/testing-tmp</test.tmp.dir>
<zstd-jni.version>1.5.6-4</zstd-jni.version>
<jts.version>1.19.0</jts.version>
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be too old because it's released on Jun 21, 2022.

Why don't we use the latest one? Is there any incompatibility issue with something?

@dongjoon-hyun dongjoon-hyun added this to the 2.1.0 milestone Sep 25, 2024
@wgtmac
Copy link
Member

wgtmac commented Sep 25, 2024

apache/parquet-format#240 is still under review and subject to change. I think we can wait until it has been finalized. @dongjoon-hyun

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Sep 25, 2024

Ya, I want to check ETA for this work. Do you think we can have this for Apache ORC 2.1?

@wgtmac
Copy link
Member

wgtmac commented Sep 25, 2024

If 2.1.0 will be released around January 17, 2025, then I think yes.

@dongjoon-hyun
Copy link
Member

Happy New Year, @ffacs and @wgtmac .

We need to release Apache ORC 2.1.0 this month.

Given that the status of Apache Parquet community, it's not finished still, right?

Let me remove this from ORC 2.1.0 milestone for now, @wgtmac .

@dongjoon-hyun dongjoon-hyun removed this from the 2.1.0 milestone Jan 3, 2025
@dongjoon-hyun
Copy link
Member

For the record, we can have this at Apache ORC 3.0.0.

@wgtmac
Copy link
Member

wgtmac commented Jan 5, 2025

Yes, the discussion of Geometry type on the Iceberg and Parquet side took longer time than expected. I don't think it will be closed soon. Moving it to 3.0.0 seems reasonable.

@dongjoon-hyun
Copy link
Member

Thank you for the confirmation, @wgtmac .

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Apr 10, 2025

It seems that we are ready to release Apache ORC Format v1.1.0 to support this PR.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Apr 29, 2025

Can we resume this since the main branch is using ORC Format v1.1, @ffacs and @wgtmac ?

Apache Iceberg v1.9.0 is also released with geo type.

@ffacs
Copy link
Contributor Author

ffacs commented Apr 30, 2025

Can we resume this since the main branch is using ORC Format v1.1, @ffacs and @wgtmac ?

Apache Iceberg v1.9.0 is also released with geo type.

Yes, I'd start work on this these days.

@dongjoon-hyun
Copy link
Member

Thank you so much!

@ffacs ffacs changed the title [WIP] Add support for geometry type ORC-1903: Add support for geospatial types Jun 2, 2025
@ffacs ffacs marked this pull request as ready for review June 2, 2025 15:35
@dongjoon-hyun
Copy link
Member

Thank you for updating PR, @ffacs .

@dongjoon-hyun
Copy link
Member

Thank you for updating, @ffacs .

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you so much, @ffacs .

cc @williamhyun , @wgtmac , @cxzl25 , @guiyanakuang

and cc @MrPowers , @mrpowers-wb, @jiayuasu from #2224

Copy link
Contributor

@cxzl25 cxzl25 left a comment

Choose a reason for hiding this comment

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

LGTM

@dongjoon-hyun
Copy link
Member

Thank you, @cxzl25 .

@williamhyun
Copy link
Member

Thank you @ffacs, @dongjoon-hyun, @wgtmac, and @cxzl25!
Will be merging this now.

@dongjoon-hyun
Copy link
Member

Thank you, @williamhyun .

@ffacs
Copy link
Contributor Author

ffacs commented Jun 15, 2025

Thank you @dongjoon-hyun @williamhyun @cxzl25 .

dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Jul 30, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade ORC to 2.2.0. 2.2.0 RC1 is currently under voting.

### Why are the changes needed?

Apache ORC 2.2.0 is a new feature release.
- https://github.com/apache/orc/releases/tag/v2.2.0
  - apache/orc#2032
  - apache/orc#2338
  - apache/orc#2269
  - apache/orc#2249
  - apache/orc#2144

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #51676 from williamhyun/ORC-2.2.0.

Authored-by: William Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants