Bugfix: SQL type mapping for legacy JDBC output - #3613

Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged

Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom
Swiddis:bugfix/join-timestamps

Conversation

@Swiddis

@SwiddisSwiddis commented May 8, 2025•
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.

Related Issues

Resolves#1545
Resolves#3159

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Swiddis added 5 commits May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis mentioned this pull request May 8, 2025
Open
Swiddis
Swiddis commented May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json Outdated
{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}

@SwiddisSwiddisMay 8, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.

@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis marked this pull request as ready for review May 8, 2025 18:20
@SwiddisSwiddis added bug Something isn't working SQL labels May 8, 2025
RyanL1997
RyanL1997 approved these changes Jul 7, 2025
View reviewed changes

@RyanL1997RyanL1997 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generally LGTM.

Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
JSONObject result = executeQuery(query);
JSONArray schema = result.getJSONArray("schema");

Assert.assertFalse(schema.isEmpty());

@RyanL1997RyanL1997Jul 7, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:

privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}

@SwiddisSwiddisJul 8, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types

@opensearch-trigger-bot

opensearch-trigger-botBot commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

This PR is stalled because it has been open for 30 days with no activity.

penghuo
penghuo previously approved these changes Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997 previously approved these changes Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997 previously approved these changes Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed the bugfix/join-timestamps branch from bb07114 to f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis merged commit b049ac1 into opensearch-project:mainSep 30, 2025
33 checks passed
asifabashar added a commit to asifabashar/sql that referenced this pull request Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees

No one assigned

Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent [BUG] Inconsistent behaviour for date field in nested collection queries

5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Bugfix: SQL type mapping for legacy JDBC output - #3613
Merged
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Sep 30, 2025
Merged
Bugfix: SQL type mapping for legacy JDBC output#3613
Swiddis merged 14 commits into
opensearch-project:mainfrom 
Swiddis:bugfix/join-timestamps
Conversation
@Swiddis
@SwiddisSwiddis
commented
May 8, 2025•

edited

Loading

Copy link


Copy Markdown

Collaborator
Description
Another Legacy-V2 mismatch issue: Legacy doesn't apply type mappings that V2 applies, which causes inconsistency with the mapping described at https://github.com/opensearch-project/sql/blob/main/docs/user/general/datatypes.rst#data-types-mapping. This PR adds this mapping at the serialization stage, making the two engines return consistent data types.
Related Issues
Resolves#1545
Resolves#3159
Check List
  •  New functionality includes testing.
  •  New functionality has been documented.
  •  New functionality has javadoc added.
  •  New functionality has a user manual doc added.
  •  API changes companion pull request created.
  •  Commits are signed per the DCO using --signoff.
  •  Public documentation issue/PR created.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Swiddis
added 5 commits
May 6, 2025 20:27
@Swiddis
Add reproducers for 1545, 3159, 3204
b43233c
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Undo collapse to star import
4bec578
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Fix timestamp comparison query
aa66519
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Ignore 3204 test
4a83cd7
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Hack Protocol to fix the timestamp type issue
e1e4f13
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@SwiddisSwiddis
mentioned this pull request
May 8, 2025

Open

Swiddis
Swiddis
commented
May 8, 2025
View reviewed changes
Comment threadinteg-test/src/test/resources/datetime.json
Outdated

{"login_time":"2015-01-01T12:10:30Z"}
{"index":{"_id":"3"}}
{"login_time":"1585882955"}
{"login_time":"1585882955000"}
@SwiddisSwiddisMay 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

I found while writing the reproducers that this was getting parsed as milliseconds instead of seconds, despite the magnitude. That gets sent to January 19, 1970 instead of (probably intended) April 3, 2020. I decided to update the row.
@Swiddis
Apply spotless
f0653bb
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
@Swiddis
Swiddis
marked this pull request as ready for review
May 8, 2025 18:20
@SwiddisSwiddis
added

bug
Something isn't working
SQL

labels
May 8, 2025
RyanL1997
RyanL1997
approved these changes
Jul 7, 2025
View reviewed changes
@RyanL1997RyanL1997
left a comment


Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Generally LGTM.
Comment threadinteg-test/src/test/java/org/opensearch/sql/sql/ComplexTimestampQueryIT.java
 JSONObject result = executeQuery(query);
 JSONArray schema = result.getJSONArray("schema");

 Assert.assertFalse(schema.isEmpty());
@RyanL1997RyanL1997Jul 7, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

nit - Consider parameterizing the schema type check logic to reduce duplication:
privatevoidassertSchemaTypeIsTimestamp(JSONArrayschema) {
Assert.assertFalse(schema.isEmpty());
for (inti = 0; i < schema.length(); i++) {
Assert.assertEquals("timestamp", schema.getJSONObject(i).getString("type"));
}
}
@SwiddisSwiddisJul 8, 2025

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Probably not enough duplication here to warrant (this is a bit of a special case for type bugs), but the robust solution would be to make a custom schema assertion -- assertSchemaMatches(schema, ["timestamp", "timestamp"]). Then we throw it behind a custom assertion error that indicates the mismatching data types
@opensearch-trigger-bot
opensearch-trigger-botBot
commented
Aug 15, 2025

Copy link


Copy Markdown

Contributor
This PR is stalled because it has been open for 30 days with no activity.
penghuo
penghuo
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Aug 20, 2025
View reviewed changes
RyanL1997
RyanL1997
previously approved these changes
Sep 17, 2025
View reviewed changes
@Swiddis
Swiddisforce-pushed
the
bugfix/join-timestamps
branch
from
bb07114 to
f52c763CompareSeptember 17, 2025 16:54
@Swiddis
Merge branch 'main' into bugfix/join-timestamps
4ba80b9
Signed-off-by: Simeon Widdis <sawiddis@gmail.com>
@Swiddis
Swiddis
merged commit b049ac1
into
opensearch-project:mainSep 30, 2025

33 checks passed

asifabashar added a commit
to asifabashar/sql
that referenced
this pull request
Oct 10, 2025
@asifabashar
Merge branch 'main-apple' into sync-mirror-update
6505a4e
* main-apple: (218 commits)
Add ignorePrometheus Flag for integTest and docTest (opensearch-project#4442)
Create fab-radar.yml
PPL `fillnull` command enhancement (opensearch-project#4421)
reverting to _doc + _id (opensearch-project#4435)
Support `multisearch` command in calcite (opensearch-project#4332)
Add 3.3 release notes (opensearch-project#4422) (opensearch-project#4423)
[SQL/PPL] Fix the `count(*)` and `dc(field)` to be capped at MAX_INTEGER opensearch-project#4416 (opensearch-project#4418)
Change the default search sort tiebreaker to `_shard_doc` for PIT search (opensearch-project#4378)
[Enhancement] Add error handling for known limitation of sql `JOIN` (opensearch-project#4344)
Bugfix: SQL type mapping for legacy JDBC output (opensearch-project#3613)
Version bump: 3.3 (opensearch-project#4417)
Add max/min eval functions (opensearch-project#4333)
Support time modifiers in search command (opensearch-project#4224)
Fix numbered token bug and make it optional output in patterns command (opensearch-project#4402)
refactor span (opensearch-project#4334)
Move release notes categories (opensearch-project#3818)
[Doc] Enable doctest with Calcite (opensearch-project#4379)
Mod function should return decimal instead of float when handle the operands are decimal literal (opensearch-project#4407)
Scale of decimal literal should always be positive in Calcite (opensearch-project#4401)
Enable Calcite by default and implicit fallback the unsupported commands (opensearch-project#4372)
...
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@penghuopenghuopenghuo approved these changes
@RyanL1997RyanL1997RyanL1997 approved these changes
@ps48ps48Awaiting requested review from ps48ps48 is a code owner
@kavithacmkavithacmAwaiting requested review from kavithacm
@derek-hoderek-hoAwaiting requested review from derek-ho
@joshuali925joshuali925Awaiting requested review from joshuali925joshuali925 is a code owner
@YANG-DBYANG-DBAwaiting requested review from YANG-DB
@mengweiericmengweiericAwaiting requested review from mengweiericmengweieric is a code owner
@seankao-azseankao-azAwaiting requested review from seankao-az
@MaxKsyunzMaxKsyunzAwaiting requested review from MaxKsyunz
@Yury-FridlyandYury-FridlyandAwaiting requested review from Yury-Fridlyand
@anirudhaanirudhaAwaiting requested review from anirudhaanirudha is a code owner
@forestmveyforestmveyAwaiting requested review from forestmvey
@acarbonettoacarbonettoAwaiting requested review from acarbonettoacarbonetto is a code owner
@GumpacGGumpacGAwaiting requested review from GumpacG
@ykmr1224ykmr1224Awaiting requested review from ykmr1224ykmr1224 is a code owner
@LantaoJinLantaoJinAwaiting requested review from LantaoJinLantaoJin is a code owner
@noChargernoChargerAwaiting requested review from noChargernoCharger is a code owner
@qianheng-awsqianheng-awsAwaiting requested review from qianheng-awsqianheng-aws is a code owner
@anasalkouzanasalkouzAwaiting requested review from anasalkouz
@dai-chendai-chenAwaiting requested review from dai-chendai-chen is a code owner
@vamsimanoharvamsimanoharAwaiting requested review from vamsimanoharvamsimanohar is a code owner
@yuancuyuancuAwaiting requested review from yuancuyuancu is a code owner

Assignees


No one assigned


Labels

bugSomething isn't workinglegacyIssues related to legacy query engine to be deprecatedSQLv3.3.0

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[BUG] timestamp data type handling is inconsistent

[BUG] Inconsistent behaviour for date field in nested collection queries


5 participants

@Swiddis@penghuo@anasalkouz@dai-chen@RyanL1997