Skip to content

Conversation

@amotl
Copy link
Contributor

@amotl amotl commented Dec 21, 2023

About

The new SQLAlchemy dialect for CrateDB includes the FloatVector type. This patch intends to validate that everything works well, and removes the previous implementation, which originally was conceived here.

References

/cc @hammerhead, @surister, @ckurze

@codecov
Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (582b512) 86.11% compared to head (0ec842a) 88.87%.

Files Patch % Lines
target_cratedb/connector.py 80.00% 1 Missing ⚠️
target_cratedb/sqlalchemy/patch.py 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
+ Coverage   86.11%   88.87%   +2.75%     
==========================================
  Files           7        6       -1     
  Lines         713      665      -48     
==========================================
- Hits          614      591      -23     
+ Misses         99       74      -25     
Flag Coverage Δ
main 88.87% <88.88%> (+2.75%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pyproject.toml Outdated
"cratedb-toolkit",
'importlib-resources; python_version < "3.9"', # "meltanolabs-target-postgres==0.0.9",
"meltanolabs-target-postgres@ git+https://github.com/singer-contrib/meltanolabs-target-postgres.git@pgvector",
"sqlalchemy-cratedb[vector]@ git+https://github.com/crate-workbench/sqlalchemy-cratedb@amo/type-float-vector",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, both of them need to be released before a serious GA release of meltano-target-cratedb.

@amotl amotl marked this pull request as ready for review December 21, 2023 22:11
@amotl amotl moved this to Todo in Workbench Overview Mar 29, 2025
@amotl amotl force-pushed the use-sqlalchemy-dialect-ng branch 5 times, most recently from 0c5d1ba to 90a88a6 Compare November 10, 2025 16:00
@amotl amotl requested review from matriv and seut and removed request for ckurze, hammerhead and surister November 10, 2025 16:21
This includes the `FloatVector` SQLAlchemy type.
@amotl amotl force-pushed the use-sqlalchemy-dialect-ng branch from 90a88a6 to 148a2d5 Compare November 11, 2025 11:21
TYPES_MAP["real_array"] = ARRAY(sqltypes.DOUBLE)
TYPES_MAP["timestamp without time zone"] = sqltypes.TIMESTAMP
TYPES_MAP["timestamp with time zone"] = sqltypes.TIMESTAMP
# abc()
Copy link

Choose a reason for hiding this comment

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

remove? Otherwise, what does this mean?

Comment on lines -61 to +69
json_encoder_default = CrateJsonEncoder.default
json_encoder_default = crate.client.http.json_encoder

def default(self, o):
if isinstance(o, Decimal):
return float(o)
return json_encoder_default(o)
def json_encoder_new(obj: Any) -> Union[int, str, float]:
if isinstance(obj, Decimal):
return float(obj)
return json_encoder_default(obj)

CrateJsonEncoder.default = default
crate.client.http.json_encoder = json_encoder_new
Copy link
Contributor Author

@amotl amotl Nov 22, 2025

Choose a reason for hiding this comment

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

That patch should go into crate/crate-python instead?

Copy link
Contributor Author

@amotl amotl Nov 22, 2025

Choose a reason for hiding this comment

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

Hm. We are still in a limbo whether to forward Decimal type values as float or string?

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

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants