Skip to content

Commit

Permalink
fix(oracle): disable dynamic limits
Browse files Browse the repository at this point in the history
There is working code for this, but the pairing of dynamic limits with
no offsets seems counterintuitive.
  • Loading branch information
gforsyth committed Jan 19, 2024
1 parent c1d6d03 commit 65eb3ca
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
9 changes: 7 additions & 2 deletions ibis/backends/oracle/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import ibis
import ibis.common.exceptions as com
import ibis.expr.operations as ops
from ibis.backends.base.sqlglot.compiler import NULL, STAR, C, SQLGlotCompiler
from ibis.backends.base.sqlglot.compiler import NULL, STAR, SQLGlotCompiler
from ibis.backends.base.sqlglot.datatypes import OracleType
from ibis.backends.base.sqlglot.rewrites import Window, replace_log2, replace_log10
from ibis.common.patterns import replace
Expand Down Expand Up @@ -207,7 +207,12 @@ def visit_Limit(self, op, *, parent, n, offset):
if isinstance(n, int):
result = result.limit(n)

Check warning on line 208 in ibis/backends/oracle/compiler.py

View check run for this annotation

Codecov / codecov/patch

ibis/backends/oracle/compiler.py#L208

Added line #L208 was not covered by tests
elif n is not None:
result = result.where(C.ROWNUM <= sg.select(n).from_(parent).subquery())
raise com.UnsupportedArgumentError(

Check warning on line 210 in ibis/backends/oracle/compiler.py

View check run for this annotation

Codecov / codecov/patch

ibis/backends/oracle/compiler.py#L210

Added line #L210 was not covered by tests
"No support for dynamic limit in the Oracle backend."
)
# TODO: re-enable this for dynamic limits
# but it should be paired with offsets working
# result = result.where(C.ROWNUM <= sg.select(n).from_(parent).subquery())
else:
assert n is None, n

Check warning on line 217 in ibis/backends/oracle/compiler.py

View check run for this annotation

Codecov / codecov/patch

ibis/backends/oracle/compiler.py#L217

Added line #L217 was not covered by tests
if self.no_limit_value is not None:
Expand Down
11 changes: 10 additions & 1 deletion ibis/backends/tests/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,7 @@ def test_try_cast_func(con, from_val, to_type, func):
raises=ExaQueryError,
reason="doesn't support OFFSET without ORDER BY",
),
pytest.mark.notyet(["oracle"], raises=com.UnsupportedArgumentError),
],
),
param(
Expand All @@ -1431,6 +1432,7 @@ def test_try_cast_func(con, from_val, to_type, func):
raises=ImpalaHiveServer2Error,
reason="impala doesn't support OFFSET without ORDER BY",
),
pytest.mark.notyet(["oracle"], raises=com.UnsupportedArgumentError),
],
),
# positive stop
Expand All @@ -1449,6 +1451,7 @@ def test_try_cast_func(con, from_val, to_type, func):
raises=ExaQueryError,
reason="doesn't support OFFSET without ORDER BY",
),
pytest.mark.notyet(["oracle"], raises=com.UnsupportedArgumentError),
],
),
param(
Expand All @@ -1462,6 +1465,7 @@ def test_try_cast_func(con, from_val, to_type, func):
reason="mssql doesn't support OFFSET without LIMIT",
),
pytest.mark.notyet(["exasol"], raises=ExaQueryError),
pytest.mark.notyet(["oracle"], raises=com.UnsupportedArgumentError),
pytest.mark.notyet(
["impala"],
raises=ImpalaHiveServer2Error,
Expand Down Expand Up @@ -1518,7 +1522,7 @@ def test_static_table_slice(backend, slc, expected_count_fn):
@pytest.mark.notyet(
["oracle"],
raises=com.UnsupportedArgumentError,
reason="backend doesn't support dynamic limit/offset",
reason="Removed half-baked dynamic offset functionality for now",
)
@pytest.mark.notyet(
["trino"],
Expand Down Expand Up @@ -1575,6 +1579,11 @@ def test_dynamic_table_slice(backend, slc, expected_count_fn):
raises=SnowflakeProgrammingError,
reason="backend doesn't support dynamic limit/offset",
)
@pytest.mark.notyet(
["oracle"],
raises=com.UnsupportedArgumentError,
reason="Removed half-baked dynamic offset functionality for now",
)
@pytest.mark.notimpl(
["trino"],
raises=TrinoUserError,
Expand Down

0 comments on commit 65eb3ca

Please sign in to comment.