Skip to content

Commit

Permalink
Merge pull request #1336 from mabel-dev/#1335
Browse files Browse the repository at this point in the history
  • Loading branch information
joocer authored Dec 14, 2023
2 parents eec2cec + fc49cfd commit 3e77979
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 8 deletions.
2 changes: 1 addition & 1 deletion opteryx/__version__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
__build__ = 130
__build__ = 134
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand Down
10 changes: 10 additions & 0 deletions opteryx/components/binder/binder.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,16 @@ def inner_binder(node: Node, context: Dict[str, Any]) -> Tuple[Node, Dict[str, A
node.query_column = node.alias or column_name

else:
from opteryx.components.binder.binder_visitor import (
get_mismatched_condition_column_types,
)

mismatches = get_mismatched_condition_column_types(node, relaxed_numeric=True)
if mismatches:
from opteryx.exceptions import IncompatibleTypesError

raise IncompatibleTypesError(**mismatches)

schema_column = ExpressionColumn(
name=column_name,
aliases=[node.alias] if node.alias else [],
Expand Down
27 changes: 21 additions & 6 deletions opteryx/components/binder/binder_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
CAMEL_TO_SNAKE = re.compile(r"(?<!^)(?=[A-Z])")


def get_mismatched_condition_column_types(node: Node) -> dict:
def get_mismatched_condition_column_types(node: Node, relaxed_numeric: bool = False) -> dict:
"""
Checks that the types of the fields involved a comparison are the same on both sides.
Expand All @@ -50,21 +50,29 @@ def get_mismatched_condition_column_types(node: Node) -> dict:
Returns:
a dictionary describing the columns
"""
if node.node_type == NodeType.AND:
left_mismatches = get_mismatched_condition_column_types(node.left)
right_mismatches = get_mismatched_condition_column_types(node.right)
if node.node_type in (NodeType.AND, NodeType.OR):
left_mismatches = get_mismatched_condition_column_types(node.left, relaxed_numeric)
right_mismatches = get_mismatched_condition_column_types(node.right, relaxed_numeric)
return left_mismatches or right_mismatches

elif node.node_type == NodeType.COMPARISON_OPERATOR:
left_type = node.left.schema_column.type
right_type = node.right.schema_column.type
if node.value in ("InList", "NotInList", "Contains", "NotContains"):
return None # ARRAY ops are meant to have different types
left_type = node.left.schema_column.type if node.left.schema_column else None
right_type = node.right.schema_column.type if node.right.schema_column else None

if left_type != 0 and right_type != 0 and left_type != right_type:
if relaxed_numeric and left_type.is_numeric() and right_type.is_numeric():
return None
if left_type == OrsoTypes.NULL or right_type == OrsoTypes.NULL:
return None # None comparisons are allowed
return {
"left_column": f"{node.left.source}.{node.left.value}",
"left_type": left_type.name,
"left_node": node.left,
"right_column": f"{node.right.source}.{node.right.value}",
"right_type": right_type.name,
"right_node": node.right,
}

return None # if we reach here, it means we didn't find any inconsistencies
Expand Down Expand Up @@ -106,6 +114,13 @@ def extract_join_fields(
right_fields.extend(right_fields_2)

elif condition_node.node_type == NodeType.COMPARISON_OPERATOR and condition_node.value == "Eq":
if any(
[
condition_node.left.node_type != NodeType.IDENTIFIER,
condition_node.right.node_type != NodeType.IDENTIFIER,
]
):
raise UnsupportedSyntaxError("JOIN conditions only support column comparisons.")
if (
condition_node.left.source in left_relation_names
and condition_node.right.source in right_relation_names
Expand Down
12 changes: 11 additions & 1 deletion opteryx/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
└── VariableNotFoundError
"""

from typing import Any
from typing import Optional


Expand Down Expand Up @@ -283,7 +284,16 @@ def __init__(
column: Optional[str] = None,
left_column: Optional[str] = None,
right_column: Optional[str] = None,
left_node: Optional[Any] = None,
right_node: Optional[Any] = None,
):
def _format_col(_type, _node, _name):
if _node.node_type == 42:
return f"literal '{_node.value}' ({_type})"
if _node.node_type == 38:
return f"column '{_name}' ({_type})"
return _name

self.left_type = left_type
self.right_type = right_type
self.column = column
Expand All @@ -295,7 +305,7 @@ def __init__(
)
elif self.left_column or self.right_column:
super().__init__(
f"Incompatible types for columns '{left_column}' ({left_type}) and '{right_column}' ({right_type})."
f"Incompatible types for {_format_col(left_type, left_node, left_column)} and {_format_col(right_type, right_node, right_column)}. Using `CAST(column AS type)` may help resolve."
)
else:
super().__init__("Incompatible column types.")
Expand Down
2 changes: 2 additions & 0 deletions tests/sql_battery/test_shapes_and_errors_battery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,8 @@
("SELECT * FROM $planets INNER JOIN $satellites WHERE $planets.name != $satellites.name", None, None, SqlError),
("SELECT * FROM $planets INNER JOIN $satellites ON $planets.name != $satellites.name", 0, 28, UnsupportedSyntaxError),
("SELECT a.name, b.name FROM sqlite.planets a JOIN sqlite.planets b ON a.numberOfMoons = b.numberOfMoons WHERE a.name <> b.name", 2, 2, None),
("SELECT * FROM $planets INNER JOIN $satellites ON INTEGER($planets.id) = INTEGER($satellites.planetId)", None, None, UnsupportedSyntaxError),
("SELECT alma_mater LIKE '%a%' FROM $astronauts", None, None, IncompatibleTypesError),

("SELECT VARCHAR(birth_place) FROM $astronauts", 357, 1, None),
("SELECT name FROM $astronauts WHERE GET(STRUCT(VARCHAR(birth_place)), 'state') = birth_place['state']", 357, 1, None),
Expand Down

0 comments on commit 3e77979

Please sign in to comment.