Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug][Connector][FileBase]Parquet reader parsing array type exception. #4457

Merged
merged 14 commits into from
Nov 28, 2023

Conversation

lightzhao
Copy link
Contributor

Purpose of this pull request

Parquet reader parsing array type exception. [java.lang.ArrayStoreException]

Check list

@Hisoka-X Hisoka-X requested a review from TyrantLucifer March 30, 2023 10:48
Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

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

Add ut/it cases to verify this pull request is valid

@lightzhao
Copy link
Contributor Author

((GenericData.Array<?>) field).iterator().forEachRemaining(origArray::add)
image
The modified test result is:
image

@lightzhao
Copy link
Contributor Author

@EricJoy2048 @TyrantLucifer @Hisoka-X PTAL, thanks.

@lightzhao
Copy link
Contributor Author

who has time help review, thanks.

SeaTunnelRowType seaTunnelRowTypeInfo =
parquetReadStrategy.getSeaTunnelRowTypeInfo(localConf, path);
Assertions.assertNotNull(seaTunnelRowTypeInfo);
System.out.println(seaTunnelRowTypeInfo);
Copy link
Member

Choose a reason for hiding this comment

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

Why not use assert?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done .PTAL.

@lightzhao lightzhao requested a review from EricJoy2048 May 25, 2023 02:36
@lightzhao
Copy link
Contributor Author

please approve ci.

@lightzhao
Copy link
Contributor Author

@Hisoka-X @EricJoy2048 @TyrantLucifer please approve ci.

@lightzhao
Copy link
Contributor Author

@Hisoka-X @EricJoy2048 @TyrantLucifer please approve ci.

@ic4y ic4y closed this Aug 6, 2023
@ic4y ic4y reopened this Aug 6, 2023
@ic4y
Copy link
Contributor

ic4y commented Aug 6, 2023

Waiting for CI/CD.

@lightzhao
Copy link
Contributor Author

@lightzhao
Copy link
Contributor Author

image Why is there such an exception? It has nothing to do with the module I submitted, and the latest code has also been merged.

@lightzhao
Copy link
Contributor Author

@EricJoy2048 @ic4y @TyrantLucifer PTAL.

Copy link
Member

Choose a reason for hiding this comment

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

Based on #5545, we need to remove the binary and automatically generate it

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

public static final String DATA_FILE_PATH = "/tmp/data.parquet";

public static void generateTestData() throws IOException {
System.out.println("generateTestData start.");
Copy link
Member

Choose a reason for hiding this comment

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

Please use log instead of System.out.println.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@hailin0
Copy link
Member

hailin0 commented Nov 23, 2023

LGTM

waiting for ci ok

@lightzhao
Copy link
Contributor Author

LGTM

waiting for ci ok

CI is a bit unstable, but it is successful after re-running.

@hailin0
Copy link
Member

hailin0 commented Nov 23, 2023

LGTM
waiting for ci ok

CI is a bit unstable, but it is successful after re-running.

Yes. Are you interested in helping improve it?

@Test
public void testParquetReadArray() throws Exception {
String os = System.getProperty("os.name").toLowerCase();
if (!os.contains("win")) {
Copy link
Member

Choose a reason for hiding this comment

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

You can use @DisabledOnOs(OS.WINDOWS) to instead of this. Please refer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@lightzhao
Copy link
Contributor Author

LGTM
waiting for ci ok

CI is a bit unstable, but it is successful after re-running.

Yes. Are you interested in helping improve it?

ok, i try to study it.

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

LGTM if test passed.

@Hisoka-X Hisoka-X added this to the 2.3.4 milestone Nov 23, 2023
@lightzhao
Copy link
Contributor Author

ci re-run success. @hailin0 @liugddx @ic4y PTAL.

@lightzhao
Copy link
Contributor Author

@hailin0 @EricJoy2048 PTAL.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@Hisoka-X Hisoka-X merged commit 5c6b113 into apache:dev Nov 28, 2023
5 of 6 checks passed
@lightzhao lightzhao deleted the parquet-reader-array branch November 28, 2023 07:57
alextinng pushed a commit to alextinng/seatunnel that referenced this pull request Dec 19, 2023
apache#4457)



---------

Co-authored-by: lightzhao <zhaolianyong777@gmail.com>
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.

7 participants