postgres-source: handle 24:00:00 value for time column #17782

subodh1810 · 2022-10-10T12:59:23Z

For : https://github.com/airbytehq/oncall/issues/647#issuecomment-1271442608

edgao · 2022-10-10T15:39:13Z

airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/DateTimeConverter.java

        return LocalTime.ofNanoOfDay(value).format(TIME_FORMATTER);
      } else {
-        final long updatedValue = 0 > value ? Math.abs(value) : TimeUnit.DAYS.toNanos(1);
+        final long updatedValue = 0 > value ? Math.abs(value) : 86399999999999L;


this would be more clear, assuming I understood correctly

Suggested change

final long updatedValue = 0 > value ? Math.abs(value) : 86399999999999L;

final long updatedValue = 0 > value ? Math.abs(value) : TimeUnit.DAYS.toNanos(1) - 1;

or maybe just this? I.e. convert negative values to positive, and then cap all values to less than 23:59 - this is slightly different from the current behavior, where -864000...0000 would be converted to 864000...0000 rather than being reduced to 86399999999999)

Suggested change

final long updatedValue = 0 > value ? Math.abs(value) : 86399999999999L;

final long updatedValue = Math.min(Math.abs(value), TimeUnit.DAYS.toNanos(1) - 1);

airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/DateTimeConverter.java

subodh1810 · 2022-10-10T16:23:32Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3220863395
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3220863395
No Python unittests run

Build Passed

Test summary info:

All Passed

subodh1810 · 2022-10-10T16:23:37Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3220864224
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3220864224
🐛 https://gradle.com/s/6bd7c22asn2p6

Build Failed

Test summary info:

Could not find result summary

rodireich · 2022-10-10T17:54:41Z

airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/DateTimeConverter.java

+      final String valueAsString = time.toString();
+      if (valueAsString.startsWith("24")) {
+        LOGGER.debug("Time value {} is above range, converting to 23:59:59", valueAsString);
+        return LocalTime.parse("23:59:59.999999").format(TIME_FORMATTER);


LocalTime has .MAX

subodh1810 · 2022-10-10T18:04:44Z

airbyte-integrations/connectors/source-postgres/src/test/resources/expected_spec.json

@@ -133,23 +133,23 @@
              },
              "client_certificate": {
                "type": "string",
-                "title": "Client Certificate (Optional)",
+                "title": "Client Certificate",


The changes to this file were required for PR #17544

subodh1810 · 2022-10-10T18:05:22Z

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3221415092
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/3221415092
No Python unittests run

Build Passed

Test summary info:

All Passed

subodh1810 · 2022-10-10T18:05:32Z

/test connector=connectors/source-mysql

🕑 connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3221415646
✅ connectors/source-mysql https://github.com/airbytehq/airbyte/actions/runs/3221415646
No Python unittests run

Build Passed

Test summary info:

All Passed

akashkulk · 2022-10-10T18:42:04Z

airbyte-db/db-lib/src/main/java/io/airbyte/db/jdbc/DateTimeConverter.java

        return LocalTime.ofNanoOfDay(value).format(TIME_FORMATTER);
      } else {
-        final long updatedValue = 0 > value ? Math.abs(value) : TimeUnit.DAYS.toNanos(1);
+        final long updatedValue = Math.min(Math.abs(value), LocalTime.MAX.toNanoOfDay());
        LOGGER.debug("Time values must use number of milliseconds greater than 0 and less than 86400000000000 but its {}, converting to {} ", value,


Should this comment say nanoseconds instead?

akashkulk · 2022-10-10T18:46:15Z

Could you add some brief context to the PR description?

subodh1810 · 2022-10-10T20:42:28Z

/publish connector=connectors/source-postgres

🕑 Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/3222241467

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres	❌	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

github-actions · 2022-10-11T11:39:49Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-postgres-strict-encrypt
source-alloydb
source-alloydb-strict-encrypt

subodh1810 · 2022-10-11T11:39:50Z

/publish connector=connectors/source-postgres

🕑 Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/3226515657

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres	❌	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

subodh1810 · 2022-10-11T11:39:58Z

/publish connector=connectors/source-postgres-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-postgres-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3226516290

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres-strict-encrypt	❌	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

subodh1810 · 2022-10-11T19:35:28Z

/publish connector=connectors/source-postgres

🕑 Publishing the following connectors:
connectors/source-postgres
https://github.com/airbytehq/airbyte/actions/runs/3229710852

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

subodh1810 · 2022-10-11T19:35:36Z

/publish connector=connectors/source-postgres-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-postgres-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3229711576

Connector	Did it publish?	Were definitions generated?
connectors/source-postgres-strict-encrypt	✅	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

github-actions · 2022-10-11T19:35:57Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-alloydb
source-postgres-strict-encrypt
source-alloydb-strict-encrypt

github-actions · 2022-10-11T19:41:50Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-alloydb
source-postgres-strict-encrypt
source-alloydb-strict-encrypt

subodh1810 · 2022-10-11T19:42:14Z

/publish connector=connectors/source-alloydb

🕑 Publishing the following connectors:
connectors/source-alloydb
https://github.com/airbytehq/airbyte/actions/runs/3229749736

Connector	Did it publish?	Were definitions generated?
connectors/source-alloydb	✅	✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

subodh1810 · 2022-10-11T19:42:29Z

/publish connector=connectors/source-alloydb-strict-encrypt

🕑 Publishing the following connectors:
connectors/source-alloydb-strict-encrypt
https://github.com/airbytehq/airbyte/actions/runs/3229751328

Connector	Did it publish?	Were definitions generated?
connectors/source-alloydb-strict-encrypt	✅	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

…bytehq/airbyte into handle-24-hour-time-postgres

github-actions · 2022-10-11T20:43:26Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

source-alloydb
source-alloydb-strict-encrypt
source-postgres-strict-encrypt

* postgres-source: handle 24:00:00 value for time column * address review comment * fix test + use LocalTime.MAX * bump version * update alloy db as well * auto-bump connector version [ci skip] * auto-bump connector version [ci skip] Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

postgres-source: handle 24:00:00 value for time column

c9fdbe3

subodh1810 requested a review from edgao October 10, 2022 12:59

subodh1810 requested a review from a team as a code owner October 10, 2022 12:59

subodh1810 self-assigned this Oct 10, 2022

github-actions bot added the area/connectors Connector related issues label Oct 10, 2022

octavia-squidington-iv added the connectors/source/postgres label Oct 10, 2022

subodh1810 temporarily deployed to more-secrets October 10, 2022 13:01 Inactive

edgao reviewed Oct 10, 2022

View reviewed changes

subodh1810 added 2 commits October 10, 2022 21:50

Merge branch 'master' into handle-24-hour-time-postgres

a8d4918

address review comment

9a3d3c7

subodh1810 requested review from edgao, akashkulk and rodireich October 10, 2022 16:22

subodh1810 temporarily deployed to more-secrets October 10, 2022 16:24 Inactive

edgao approved these changes Oct 10, 2022

View reviewed changes

rodireich reviewed Oct 10, 2022

View reviewed changes

rodireich approved these changes Oct 10, 2022

View reviewed changes

fix test + use LocalTime.MAX

4426b0b

subodh1810 commented Oct 10, 2022

View reviewed changes

subodh1810 temporarily deployed to more-secrets October 10, 2022 18:06 Inactive

akashkulk reviewed Oct 10, 2022

View reviewed changes

akashkulk approved these changes Oct 10, 2022

View reviewed changes

bump version

eab2303

github-actions bot added the area/documentation Improvements or additions to documentation label Oct 10, 2022

subodh1810 temporarily deployed to more-secrets October 10, 2022 20:44 Inactive

Merge branch 'master' into handle-24-hour-time-postgres

8fd1485

subodh1810 temporarily deployed to more-secrets October 11, 2022 11:41 Inactive

Merge branch 'master' into handle-24-hour-time-postgres

46d3d44

subodh1810 temporarily deployed to more-secrets October 11, 2022 19:37 Inactive

update alloy db as well

b302266

octavia-squidington-iv added connectors/source/alloydb connectors/source/alloydb-strict-encrypt labels Oct 11, 2022

subodh1810 temporarily deployed to more-secrets October 11, 2022 19:43 Inactive

octavia-squidington-iii added 3 commits October 11, 2022 19:52

auto-bump connector version [ci skip]

db24e7c

auto-bump connector version [ci skip]

7b64eef

Merge branch 'handle-24-hour-time-postgres' of https://github.com/air…

74222b2

…bytehq/airbyte into handle-24-hour-time-postgres

octavia-squidington-iii temporarily deployed to more-secrets October 11, 2022 20:44 Inactive

subodh1810 merged commit 4652173 into master Oct 11, 2022

subodh1810 deleted the handle-24-hour-time-postgres branch October 11, 2022 22:03

This was referenced Oct 13, 2022

Bump Airbyte version from 0.40.14 to 0.40.15 #17961

Closed

Bump Airbyte version from 0.40.14 to 0.40.15 #17970

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

postgres-source: handle 24:00:00 value for time column #17782

postgres-source: handle 24:00:00 value for time column #17782

subodh1810 commented Oct 10, 2022

edgao Oct 10, 2022

subodh1810 Oct 10, 2022

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

rodireich Oct 10, 2022 •

edited

Loading

subodh1810 Oct 10, 2022

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

akashkulk Oct 10, 2022

akashkulk commented Oct 10, 2022

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Oct 11, 2022

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Oct 11, 2022

github-actions bot commented Oct 11, 2022

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Oct 11, 2022

	final long updatedValue = 0 > value ? Math.abs(value) : 86399999999999L;
	final long updatedValue = 0 > value ? Math.abs(value) : TimeUnit.DAYS.toNanos(1) - 1;

	final long updatedValue = 0 > value ? Math.abs(value) : 86399999999999L;
	final long updatedValue = Math.min(Math.abs(value), TimeUnit.DAYS.toNanos(1) - 1);

postgres-source: handle 24:00:00 value for time column #17782

postgres-source: handle 24:00:00 value for time column #17782

Conversation

subodh1810 commented Oct 10, 2022

edgao Oct 10, 2022

Choose a reason for hiding this comment

subodh1810 Oct 10, 2022

Choose a reason for hiding this comment

subodh1810 commented Oct 10, 2022 • edited by github-actions bot Loading

Build Passed

subodh1810 commented Oct 10, 2022 • edited by github-actions bot Loading

Build Failed

rodireich Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

subodh1810 Oct 10, 2022

Choose a reason for hiding this comment

subodh1810 commented Oct 10, 2022 • edited by github-actions bot Loading

Build Passed

subodh1810 commented Oct 10, 2022 • edited by github-actions bot Loading

Build Passed

akashkulk Oct 10, 2022

Choose a reason for hiding this comment

akashkulk commented Oct 10, 2022

subodh1810 commented Oct 10, 2022 • edited by github-actions bot Loading

github-actions bot commented Oct 11, 2022

subodh1810 commented Oct 11, 2022 • edited by github-actions bot Loading

subodh1810 commented Oct 11, 2022 • edited by github-actions bot Loading

subodh1810 commented Oct 11, 2022 • edited by github-actions bot Loading

subodh1810 commented Oct 11, 2022 • edited by github-actions bot Loading

github-actions bot commented Oct 11, 2022

github-actions bot commented Oct 11, 2022

subodh1810 commented Oct 11, 2022 • edited by github-actions bot Loading

subodh1810 commented Oct 11, 2022 • edited by github-actions bot Loading

github-actions bot commented Oct 11, 2022

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

rodireich Oct 10, 2022 •

edited

Loading

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 10, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading

subodh1810 commented Oct 11, 2022 •

edited by github-actions bot

Loading