-
Notifications
You must be signed in to change notification settings - Fork 8
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
Specific integer types and limit-offset support #27
Merged
rekby
merged 14 commits into
ydb-platform:main
from
LuckySting:limit-clause-implementation
Feb 6, 2024
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
bea552b
Add optional type casting to limit-offset clause
LuckySting a576a67
Add test of types
LuckySting bf1a579
Always skip cast_to type
LuckySting dd91359
Add .vscode to .gitignore
LuckySting 9bd6243
Migrate to class based dbapi
LuckySting b521535
Implement AsyncCursor
LuckySting 7197868
Implement Async driver + tests
LuckySting 4a1edac
Add isort
LuckySting 41eb884
Fix typo
LuckySting 946a3f1
Provide tx_mode to one-time transactions
LuckySting fc9f8fb
Add original error to dbapi exceptions
LuckySting 80a61f1
Resolve rebase conflicts
LuckySting d35d524
Merge branch 'main' into limit-clause-implementation
LuckySting 4064f82
Add missing import
LuckySting File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
""" | ||
import collections | ||
import collections.abc | ||
from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple, Union | ||
from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple, Type, Union | ||
|
||
import sqlalchemy as sa | ||
import ydb | ||
|
@@ -87,15 +87,30 @@ def visit_FLOAT(self, type_: sa.FLOAT, **kw): | |
def visit_BOOLEAN(self, type_: sa.BOOLEAN, **kw): | ||
return "BOOL" | ||
|
||
def visit_uint64(self, type_: types.UInt64, **kw): | ||
return "UInt64" | ||
|
||
def visit_uint32(self, type_: types.UInt32, **kw): | ||
return "UInt32" | ||
|
||
def visit_uint64(self, type_: types.UInt64, **kw): | ||
return "UInt64" | ||
def visit_uint16(self, type_: types.UInt16, **kw): | ||
return "UInt16" | ||
|
||
def visit_uint8(self, type_: types.UInt8, **kw): | ||
return "UInt8" | ||
|
||
def visit_int64(self, type_: types.Int64, **kw): | ||
return "Int64" | ||
|
||
def visit_int32(self, type_: types.Int32, **kw): | ||
return "Int32" | ||
|
||
def visit_int16(self, type_: types.Int16, **kw): | ||
return "Int16" | ||
|
||
def visit_int8(self, type_: types.Int8, **kw): | ||
return "Int8" | ||
|
||
def visit_INTEGER(self, type_: sa.INTEGER, **kw): | ||
return "Int64" | ||
|
||
|
@@ -134,8 +149,28 @@ def get_ydb_type( | |
|
||
if isinstance(type_, (sa.Text, sa.String, sa.Uuid)): | ||
ydb_type = ydb.PrimitiveType.Utf8 | ||
|
||
# Integers | ||
elif isinstance(type_, types.UInt64): | ||
ydb_type = ydb.PrimitiveType.Uint64 | ||
elif isinstance(type_, types.UInt32): | ||
ydb_type = ydb.PrimitiveType.Uint32 | ||
elif isinstance(type_, types.UInt16): | ||
ydb_type = ydb.PrimitiveType.Uint16 | ||
elif isinstance(type_, types.UInt8): | ||
ydb_type = ydb.PrimitiveType.Uint8 | ||
elif isinstance(type_, types.Int64): | ||
ydb_type = ydb.PrimitiveType.Int64 | ||
elif isinstance(type_, types.Int32): | ||
ydb_type = ydb.PrimitiveType.Int32 | ||
elif isinstance(type_, types.Int16): | ||
ydb_type = ydb.PrimitiveType.Int16 | ||
elif isinstance(type_, types.Int8): | ||
ydb_type = ydb.PrimitiveType.Int8 | ||
elif isinstance(type_, sa.Integer): | ||
ydb_type = ydb.PrimitiveType.Int64 | ||
# Integers | ||
|
||
elif isinstance(type_, sa.JSON): | ||
ydb_type = ydb.PrimitiveType.Json | ||
elif isinstance(type_, sa.DateTime): | ||
|
@@ -188,6 +223,36 @@ def group_by_clause(self, select, **kw): | |
kw.update(within_columns_clause=True) | ||
return super(YqlCompiler, self).group_by_clause(select, **kw) | ||
|
||
def limit_clause(self, select, **kw): | ||
text = "" | ||
if select._limit_clause is not None: | ||
limit_clause = self._maybe_cast( | ||
select._limit_clause, types.UInt64, skip_types=(types.UInt64, types.UInt32, types.UInt16, types.UInt8) | ||
) | ||
text += "\n LIMIT " + self.process(limit_clause, **kw) | ||
if select._offset_clause is not None: | ||
offset_clause = self._maybe_cast( | ||
select._offset_clause, types.UInt64, skip_types=(types.UInt64, types.UInt32, types.UInt16, types.UInt8) | ||
) | ||
if select._limit_clause is None: | ||
text += "\n LIMIT NULL" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need explicit LIMIT NULL? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed in offline: it is needed to use OFFSET without LIMIT |
||
text += " OFFSET " + self.process(offset_clause, **kw) | ||
return text | ||
|
||
def _maybe_cast( | ||
self, | ||
element: Any, | ||
cast_to: Type[sa.types.TypeEngine], | ||
skip_types: Optional[Tuple[Type[sa.types.TypeEngine], ...]] = None, | ||
) -> Any: | ||
if not skip_types: | ||
skip_types = (cast_to,) | ||
if cast_to not in skip_types: | ||
skip_types = (*skip_types, cast_to) | ||
if not hasattr(element, "type") or not isinstance(element.type, skip_types): | ||
return sa.Cast(element, cast_to) | ||
return element | ||
|
||
def render_literal_value(self, value, type_): | ||
if isinstance(value, str): | ||
value = "".join(STR_QUOTE_MAP.get(x, x) for x in value) | ||
|
@@ -277,16 +342,14 @@ def _is_bound_to_nullable_column(self, bind_name: str) -> bool: | |
def _guess_bound_variable_type_by_parameters( | ||
self, bind: sa.BindParameter, post_compile_bind_values: list | ||
) -> Optional[sa.types.TypeEngine]: | ||
if not bind.expanding: | ||
if isinstance(bind.type, sa.types.NullType): | ||
return None | ||
bind_type = bind.type | ||
else: | ||
bind_type = bind.type | ||
if bind.expanding or (isinstance(bind.type, sa.types.NullType) and post_compile_bind_values): | ||
not_null_values = [v for v in post_compile_bind_values if v is not None] | ||
if not_null_values: | ||
bind_type = sa.BindParameter("", not_null_values[0]).type | ||
else: | ||
return None | ||
|
||
if isinstance(bind_type, sa.types.NullType): | ||
return None | ||
|
||
return bind_type | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a doubt about
visit_uuid
and implicit convert uuid to UTF8: YDB has own uuid type. Now it can't stored to a table, but it can be (and will sometime) in future. Then convert from uuid to UTF8 will see strange.I suggest to not convert uuid to UTF-8 into SDK and suggest users to the conversion in customers code. It is simple as
str(var)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed in offline: will solved in a separate PR