Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Removes ConnectorBindings in favor of Table getDatum #1568

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Conversation

RCHowell
Copy link
Member

Relevant Issues

#1496

Description

This PR removes the ConnectorBindings class to use a simpler approach of having a Table provide its Datum. This is the final connector replacement PR for values/tables along with statictype/ptype and partiqlvalue vs datum.

Other Information

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@RCHowell RCHowell requested a review from johnedquinn August 26, 2024 21:44
Copy link

CROSS-ENGINE Conformance Report ❌

BASE (LEGACY-A3B1DA0) TARGET (EVAL-A3B1DA0) +/-
% Passing 90.45% 96.62% 6.17% ✅
Passing 5333 5697 364 ✅
Failing 563 199 -364 ✅
Ignored 0 0 0 ✅
Total Tests 5896 5896 0 ✅

Testing Details

  • Base Commit: a3b1da0
  • Base Engine: LEGACY
  • Target Commit: a3b1da0
  • Target Engine: EVAL

Result Details

  • ❌ REGRESSION DETECTED. See Now Failing Tests. ❌
  • Passing in both: 5205
  • Failing in both: 71
  • PASSING in BASE but now FAILING in TARGET: 128
  • FAILING in BASE but now PASSING in TARGET: 492

Now Failing Tests ❌

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

Now Passing Tests

492 test(s) were previously failing in BASE (LEGACY-A3B1DA0) but now pass in TARGET (EVAL-A3B1DA0). Before merging, confirm they are intended to pass.

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

CROSS-COMMIT-LEGACY Conformance Report ✅

BASE (LEGACY-8CB2DB7) TARGET (LEGACY-A3B1DA0) +/-
% Passing 90.45% 90.45% 0.00% ✅
Passing 5333 5333 0 ✅
Failing 563 563 0 ✅
Ignored 0 0 0 ✅
Total Tests 5896 5896 0 ✅

Testing Details

  • Base Commit: 8cb2db7
  • Base Engine: LEGACY
  • Target Commit: a3b1da0
  • Target Engine: LEGACY

Result Details

  • Passing in both: 5333
  • Failing in both: 563
  • PASSING in BASE but now FAILING in TARGET: 0
  • FAILING in BASE but now PASSING in TARGET: 0

CROSS-COMMIT-EVAL Conformance Report ✅

BASE (EVAL-8CB2DB7) TARGET (EVAL-A3B1DA0) +/-
% Passing 96.62% 96.62% 0.00% ✅
Passing 5697 5697 0 ✅
Failing 199 199 0 ✅
Ignored 0 0 0 ✅
Total Tests 5896 5896 0 ✅

Testing Details

  • Base Commit: 8cb2db7
  • Base Engine: EVAL
  • Target Commit: a3b1da0
  • Target Engine: EVAL

Result Details

  • Passing in both: 5697
  • Failing in both: 199
  • PASSING in BASE but now FAILING in TARGET: 0
  • FAILING in BASE but now PASSING in TARGET: 0

Copy link
Member

@johnedquinn johnedquinn left a comment

Choose a reason for hiding this comment

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

LGTM.

@RCHowell RCHowell merged commit 8a32dd9 into v1 Aug 27, 2024
14 checks passed
@RCHowell RCHowell deleted the v1-bindings branch August 27, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants