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

feat: add support for Ruby ActiveRecord #749

Merged
merged 24 commits into from
May 22, 2023

Conversation

olavloite
Copy link
Collaborator

@olavloite olavloite commented Mar 24, 2023

Adds experimental support for Ruby ActiveRecord 7.x. This PR adds:

  • Tests using an in-mem mock Spanner server to verify that the requests that we receive from Ruby ActiveRecord.
  • Integration tests running against a real Cloud Spanner instance.
  • A sample application using all supported data types.
  • Documentation and instructions for how to set up an application to use Ruby ActiveRecord with PGAdapter, including known limitations.

@codecov
Copy link

codecov bot commented Apr 2, 2023

Codecov Report

Merging #749 (b5c185b) into postgresql-dialect (6ef7240) will increase coverage by 3.48%.
The diff coverage is 95.74%.

@@                   Coverage Diff                    @@
##             postgresql-dialect     #749      +/-   ##
========================================================
+ Coverage                 86.32%   89.81%   +3.48%     
- Complexity                 1914     2383     +469     
========================================================
  Files                       122      131       +9     
  Lines                      6216     7942    +1726     
  Branches                    849     1141     +292     
========================================================
+ Hits                       5366     7133    +1767     
+ Misses                      605      564      -41     
  Partials                    245      245              
Flag Coverage Δ
all_tests 89.81% <95.74%> (+3.48%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../cloud/spanner/pgadapter/session/SessionState.java 94.92% <ø> (-0.11%) ⬇️
...panner/pgadapter/statements/BackendConnection.java 93.92% <ø> (+3.39%) ⬆️
...nner/pgadapter/statements/ClientSideResultSet.java 85.71% <ø> (ø)
...ud/spanner/pgadapter/statements/CopyStatement.java 95.36% <ø> (+10.51%) ⬆️
.../spanner/pgadapter/statements/CopyToStatement.java 100.00% <ø> (+4.83%) ⬆️
...nner/pgadapter/statements/DeallocateStatement.java 97.36% <ø> (+0.14%) ⬆️
...spanner/pgadapter/statements/ExecuteStatement.java 91.52% <ø> (+0.29%) ⬆️
...apter/statements/ExtendedQueryProtocolHandler.java 98.03% <ø> (+0.36%) ⬆️
...dapter/statements/IntermediatePortalStatement.java 93.75% <ø> (-2.41%) ⬇️
...pter/statements/IntermediatePreparedStatement.java 95.08% <ø> (+7.86%) ⬆️
... and 64 more

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@olavloite olavloite marked this pull request as ready for review May 17, 2023 06:09
@olavloite
Copy link
Collaborator Author

olavloite commented May 17, 2023

@palladius FYI

@olavloite olavloite requested a review from hengfengli May 17, 2023 17:08
@@ -0,0 +1,18 @@
// Copyright 2022 Google LLC
Copy link
Collaborator

Choose a reason for hiding this comment

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

nits: 2022 -> 2023

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

@hengfengli hengfengli left a comment

Choose a reason for hiding this comment

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

Generally, LGTM. Why do we need a mock server just for ruby/active record? Can't we have one for all cases?

@olavloite
Copy link
Collaborator Author

Generally, LGTM. Why do we need a mock server just for ruby/active record? Can't we have one for all cases?

Thanks for the view. We have a specific mock server for Ruby/ActiveRecord, but it is based on the generic mock server that we have for other tests:

public class AbstractRubyMockServerTest extends AbstractMockServerTest {

The reason that we have a specific one for Ruby is that the Ruby driver will execute some specific queries either when a new connection is created, or when specific methods are executed on the driver (e.g. it will check for the existence of specific types the first time those types are used). We keep these specific queries in separate mock servers per driver/ecosystem. We for example also have a similar setup for .NET, as the .NET driver also executes specific queries when some data types are used.

@olavloite olavloite enabled auto-merge (squash) May 22, 2023 11:53
@olavloite olavloite disabled auto-merge May 22, 2023 15:55
@olavloite olavloite merged commit 442e45c into postgresql-dialect May 22, 2023
25 checks passed
@olavloite olavloite deleted the ruby-active-record branch May 22, 2023 15:55
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.

None yet

2 participants