Skip to content

Conversation

@xiaochen-zhou
Copy link
Contributor

Purpose of this pull request

Add support for Time type in paimon connector

image

Does this PR introduce any user-facing change?

yes

How was this patch tested?

Add new tests

Check list

Copy link
Contributor

@corgy-w corgy-w left a comment

Choose a reason for hiding this comment

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

TKS , There seems to be only a slight problem

Comment on lines 269 to 273
return DateTimeUtils.toInternal(
org.apache.seatunnel.common.utils.DateUtils.parse(strValue));
case TIME_WITHOUT_TIME_ZONE:
return DateTimeUtils.toInternal(
org.apache.seatunnel.common.utils.TimeUtils.parse(strValue));
Copy link
Contributor

Choose a reason for hiding this comment

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

org.apache.seatunnel.common.utils. These two package prefixes do not seem to be required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

org.apache.seatunnel.common.utils. These two package prefixes do not seem to be required

done.

Comment on lines 300 to 301
} else if (expression instanceof TimeValue) {
return ((TimeValue) expression).getValue();
Copy link
Contributor

Choose a reason for hiding this comment

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

expression instanceof TimeValue repeated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

expression instanceof TimeValue repeated

done.

Copy link
Contributor

@hawk9821 hawk9821 left a comment

Choose a reason for hiding this comment

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

could you add Time type in these e2e case ?

image

@github-actions github-actions bot added the e2e label Mar 4, 2025
@xiaochen-zhou
Copy link
Contributor Author

could you add Time type in these e2e case ?

image

Done.

@xiaochen-zhou xiaochen-zhou changed the title [Feature][Connector-V2] Add support for Time type in paimon connector [Feature][Connector-V2] Suppor Time type in paimon connector Mar 5, 2025
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.

Overall LGTM. cc @hawk9821 should take a look again.

Copy link
Contributor

@hawk9821 hawk9821 left a comment

Choose a reason for hiding this comment

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

LGTM

@Hisoka-X Hisoka-X merged commit 9f1e590 into apache:dev Mar 12, 2025
5 checks passed
@xiaochen-zhou xiaochen-zhou deleted the paimon_support_time branch August 3, 2025 08:13
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.

4 participants