diff --git a/.github/scripts/check-pytest-summary.sh b/.github/scripts/check-pytest-summary.sh new file mode 100644 index 00000000..e6f48953 --- /dev/null +++ b/.github/scripts/check-pytest-summary.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash + +# Importing pylibseekdb can mask pytest's non-zero process status, so CI must +# verify the final pytest summary instead. Fail closed if the summary reports +# failures/errors or does not confirm that at least one test passed. +set -euo pipefail + +pytest_log="${1:-pytest.log}" + +if [[ ! -s "${pytest_log}" ]]; then + echo "pytest log is missing or empty: ${pytest_log}" >&2 + exit 1 +fi + +summary="$(awk 'NF { last = $0 } END { print last }' "${pytest_log}")" +failure_pattern='(^|[[:space:]])[1-9][0-9]*[[:space:]]+(failed|errors?)([,=[:space:]]|$)' +success_pattern='(^|[[:space:]])[1-9][0-9]*[[:space:]]+passed([,=[:space:]]|$)' + +if printf '%s\n' "${summary}" | grep -Eiq "${failure_pattern}"; then + echo "pytest reported failures: ${summary}" >&2 + exit 1 +fi + +if ! printf '%s\n' "${summary}" | grep -Eiq "${success_pattern}"; then + echo "could not verify a successful pytest summary: ${summary}" >&2 + exit 1 +fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be2e5e2f..8ae7c501 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: fi set -o pipefail uv run pytest tests/unit_tests/ -v --log-cli-level=${log_level} | tee pytest.log - tail -n 1 pytest.log | grep '=======' | grep 'passed' |grep -q 'failed' && exit 1 || exit 0 + bash .github/scripts/check-pytest-summary.sh pytest.log - name: Run integration tests without any test mode run: | @@ -97,7 +97,7 @@ jobs: set -o pipefail uv run pytest tests/integration_tests/ -v --log-cli-level=${log_level} \ -k "not server and not embedded and not oceanbase" | tee pytest.log - tail -n 1 pytest.log | grep '=======' | grep 'passed' |grep -q 'failed' && exit 1 || exit 0 + bash .github/scripts/check-pytest-summary.sh pytest.log integration-test: runs-on: ubuntu-latest @@ -161,4 +161,4 @@ jobs: set -o pipefail uv run pytest tests/integration_tests/ -v --log-cli-level=${log_level} \ -k "${{ matrix.test_mode }}" | tee pytest.log - tail -n 1 pytest.log | grep '=======' | grep 'passed' | grep -q 'failed' && exit 1 || exit 0 + bash .github/scripts/check-pytest-summary.sh pytest.log diff --git a/src/pyseekdb/client/client_base.py b/src/pyseekdb/client/client_base.py index 658d1234..f7b9acd2 100644 --- a/src/pyseekdb/client/client_base.py +++ b/src/pyseekdb/client/client_base.py @@ -77,6 +77,14 @@ _COLLECTION_NAME_PATTERN = re.compile(r"^[A-Za-z0-9_]+$") +# DBMS_HYBRID_SEARCH.GET_SQL may quote a JSON_EXTRACT expression as though it +# were a column identifier. Require JSON_EXTRACT to start the quoted content +# so the match cannot span from one ordinary quoted identifier to another. +_QUOTED_JSON_EXTRACT_EXPRESSION_PATTERN = re.compile( + r"`(?P\s*\(*\s*JSON_EXTRACT\s*\([^`]*\)\s*\)*\s*)`", + re.IGNORECASE, +) + # Maximum allowed length for user-facing collection names. _MAX_COLLECTION_NAME_LENGTH = 512 @@ -92,6 +100,11 @@ from .types import _NOT_PROVIDED, _NotProvided # noqa: E402, F401 +def _unquote_json_extract_expressions(query_sql: str) -> str: + """Unquote JSON_EXTRACT expressions without touching adjacent SQL identifiers.""" + return _QUOTED_JSON_EXTRACT_EXPRESSION_PATTERN.sub(r"\g", query_sql) + + def is_lakebase_version_string(version_str: str) -> bool: """Return whether a ``SELECT version()`` string identifies a LakeBase cluster.""" return _LAKEBASE_VERSION_MARKER in version_str.lower() @@ -4507,16 +4520,10 @@ def _collection_hybrid_search( # Remove any surrounding quotes if present query_sql = query_sql.strip().strip("'\"") - # OB's GET_SQL wraps field names in backticks, which turns - # `JSON_EXTRACT(metadata, '$.key')` (with or without outer - # parentheses) into a literal column name instead of a - # function call. Strip the backticks so OB evaluates the - # expression as a function call. - query_sql = re.sub( - r"`([^`]*JSON_EXTRACT[^`]*)`", - r"\1", - query_sql, - ) + # OB's GET_SQL can wrap JSON_EXTRACT expressions in backticks, which + # turns them into literal column names. Unquote only those complete + # expressions; ordinary identifiers around them must remain quoted. + query_sql = _unquote_json_extract_expressions(query_sql) # Add query hint to the generated SQL hint_sql = _query_hint_to_sql(query_hint, table_name=table_name) diff --git a/tests/unit_tests/test_ci_pytest_summary.py b/tests/unit_tests/test_ci_pytest_summary.py new file mode 100644 index 00000000..5ebefe12 --- /dev/null +++ b/tests/unit_tests/test_ci_pytest_summary.py @@ -0,0 +1,44 @@ +"""Tests for the CI fallback that validates pytest's final summary line.""" + +import subprocess +from pathlib import Path + +import pytest + +CHECK_SCRIPT = Path(__file__).parents[2] / ".github" / "scripts" / "check-pytest-summary.sh" + + +@pytest.mark.parametrize( + ("summary", "expected_returncode"), + [ + ("================ 92 passed, 1 xfailed in 1.23s ================", 0), + ("================ 92 passed in 1.23s ================\n\n \t", 0), + ("= 12 failed, 92 passed, 311 skipped, 1 xpassed in 135.47s =", 1), + ("================ 1 error, 2 passed in 0.42s ================", 1), + ("================ 5 skipped in 0.10s ================", 1), + ("test session interrupted", 1), + ], +) +def test_check_pytest_summary(tmp_path: Path, summary: str, expected_returncode: int) -> None: + pytest_log = tmp_path / "pytest.log" + pytest_log.write_text(f"test output\n{summary}\n", encoding="utf-8") + + result = subprocess.run( # noqa: S603 - execute the repository's fixed CI script + ["/bin/bash", str(CHECK_SCRIPT), str(pytest_log)], + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == expected_returncode, result.stderr + + +def test_check_pytest_summary_rejects_missing_log(tmp_path: Path) -> None: + result = subprocess.run( # noqa: S603 - execute the repository's fixed CI script + ["/bin/bash", str(CHECK_SCRIPT), str(tmp_path / "missing.log")], + check=False, + capture_output=True, + text=True, + ) + + assert result.returncode == 1 diff --git a/tests/unit_tests/test_hybrid_search_sql_rewrite.py b/tests/unit_tests/test_hybrid_search_sql_rewrite.py new file mode 100644 index 00000000..f599c619 --- /dev/null +++ b/tests/unit_tests/test_hybrid_search_sql_rewrite.py @@ -0,0 +1,41 @@ +"""Regression tests for SQL returned by DBMS_HYBRID_SEARCH.GET_SQL.""" + +import pytest + +from pyseekdb.client.client_base import _unquote_json_extract_expressions + + +@pytest.mark.parametrize( + "query_sql", + [ + ( + "SELECT MATCH(`document`) AGAINST ('machine') " + "WHERE (JSON_EXTRACT(metadata, '$.category')) = 'AI' " + "ORDER BY `_score` DESC, `__pk_increment`" + ), + ("SELECT * FROM `c$v1$test` WHERE JSON_EXTRACT(metadata, '$.score') >= 90 ORDER BY `_distance`"), + ], +) +def test_unquoted_json_extract_does_not_remove_adjacent_identifier_quotes(query_sql: str) -> None: + assert _unquote_json_extract_expressions(query_sql) == query_sql + + +@pytest.mark.parametrize( + ("query_sql", "expected"), + [ + ( + "WHERE `(JSON_EXTRACT(metadata, '$.category'))` = 'AI' ORDER BY `_score`", + "WHERE (JSON_EXTRACT(metadata, '$.category')) = 'AI' ORDER BY `_score`", + ), + ( + "WHERE (`JSON_EXTRACT(metadata, '$.score')`) >= 90 ORDER BY `_distance`", + "WHERE (JSON_EXTRACT(metadata, '$.score')) >= 90 ORDER BY `_distance`", + ), + ( + "WHERE `json_extract(metadata, '$.tag')` = 'ml'", + "WHERE json_extract(metadata, '$.tag') = 'ml'", + ), + ], +) +def test_quoted_json_extract_expression_is_unquoted(query_sql: str, expected: str) -> None: + assert _unquote_json_extract_expressions(query_sql) == expected