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.

Sign up for GitHub

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

Jump to bottom

Add Initial SQL Server support #10624

Merged

AdRiley merged 29 commits into develop from wip/adr/add-sql-server-copy

Jul 30, 2024

Member

AdRiley commented Jul 22, 2024 •

edited

Loading

Pull Request Description

Adds initial SQL Server read support

There is still a lot to do on the dialect, but would prefer to move forwards with MRs that are less than 2000 lines of code. (Apologies that it is so big, I don't think it could have been much smaller.)

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.

AdRiley added the CI: Clean build required label

AdRiley changed the title ~~Squash all commits to resolve merge conflicts~~ Add Initial SQL Server support

AdRiley force-pushed the wip/adr/add-sql-server-copy branch from 191983f to 7288d31 Compare

July 23, 2024 13:47

AdRiley marked this pull request as ready for review

July 23, 2024 13:48

AdRiley requested review from jdunkerley, radeusgd, GregoryTravis, marthasharkey, 4e6, JaroslavTulach, hubertp, Akirathan and Frizi as code owners

July 23, 2024 13:48

Akirathan approved these changes

View reviewed changes

Member

Akirathan left a comment

Java/Scala changes are fine.

radeusgd reviewed

View reviewed changes

build.sbt Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Database/0.0.0-dev/src/Internal/Base_Generator.enso Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Connection.enso Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Connection.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/SQLServer_Details.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

std-bits/database/src/main/java/org/enso/database/JDBCUtils.java Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

test/Microsoft_Tests/src/SQLServer_Spec.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

test/Microsoft_Tests/src/SQLServer_Spec.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

test/Microsoft_Tests/src/SQLServer_Spec.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Microsoft/0.0.0-dev/src/Internal/SQLServer_Type_Mapping.enso Outdated Show resolved Hide resolved

AdRiley added 14 commits

July 29, 2024 18:37


          Cleanup

c08a5be


          typo

2dcc1e2

Fix

656cde1


          Remove leftover snowflake code

3d5bccc


          Remove comment

0a75145


          Add underscore

5d2b76c


          Type cleanup


          Code review fix

cebd4a2


          Cleanup

20f4413


          Add datetime roundtrip test

adb22e5


          add comment

59b4a22


          drop

c974cc3


          Refactor

89d9002


          Refactor

d5491c1

AdRiley force-pushed the wip/adr/add-sql-server-copy branch from a36d091 to d5491c1 Compare

July 29, 2024 15:37

AdRiley added 2 commits

July 29, 2024 18:44


          Fix merge

d939943

Fix

c18dbbd

radeusgd reviewed

View reviewed changes

build/build/src/enso.rs Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

build/build/src/enso.rs Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/AWS/0.0.0-dev/src/Database/Redshift/Internal/Redshift_Dialect.enso Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

distribution/lib/Standard/Table/0.0.0-dev/src/Value_Type.enso Outdated Show resolved Hide resolved

radeusgd reviewed

View reviewed changes

test/Microsoft_Tests/src/SQLServer_Spec.enso Show resolved Hide resolved

radeusgd approved these changes

View reviewed changes

Member

radeusgd left a comment

Looks good, just a few small nits

AdRiley added 3 commits

July 29, 2024 22:36

Fix

7ede44d

fix

59fa3cb


          Add comment

a135331

Frizi approved these changes

View reviewed changes

build/build/src/sqlserver.rs

+              }
+              /// Retrieve input from asynchronous reader line by line and feed them into the given function.
+              pub async fn process_lines<R: AsyncRead + Unpin>(reader: R, f: impl Fn(String)) -> Result<R> {

Contributor

Frizi Jul 30, 2024

This function looks to be unused.

hubertp approved these changes

View reviewed changes

AdRiley merged commit 0c55248 into develop

41 checks passed

AdRiley deleted the wip/adr/add-sql-server-copy branch

July 30, 2024 10:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

radeusgd radeusgd approved these changes

Akirathan Akirathan approved these changes

Frizi Frizi approved these changes

hubertp hubertp approved these changes

jdunkerley Awaiting requested review from jdunkerley jdunkerley is a code owner

GregoryTravis Awaiting requested review from GregoryTravis GregoryTravis is a code owner

marthasharkey Awaiting requested review from marthasharkey marthasharkey is a code owner

4e6 Awaiting requested review from 4e6 4e6 is a code owner

JaroslavTulach Awaiting requested review from JaroslavTulach JaroslavTulach is a code owner

Labels

CI: Clean build required