fix(reflection): do not translate native -493 syntax errors into NoSuchTableError

[UsmanGhias]

Resolves #454.

Problem

In sqlalchemy_cubrid/dialect.py, _is_no_such_table_error() returned True immediately whenever orig.errno == -493.
In CUBRID, native code -493 represents ER_PT_SYNTAX (general parser syntax error), not a dedicated table-not-found error code. When a syntax error occurs during get_columns() or get_indexes() (e.g. ProgrammingError("Syntax error: unexpected 'SELEC'", errno=-493, sqlstate="42000")), the error boundary mistakenly converted the ProgrammingError into NoSuchTableError, hiding failed metadata queries.

Solution

• Removed bare errno == -493 evaluation from _is_no_such_table_error().
• Require a verified missing-object indicator:
  • ANSI SQLSTATE 42S02 (base table or view not found), or
  • Explicit missing-table messages ('Unknown class' or 'Table not found') across both pycubrid and CUBRIDdb error shapes (including tuple args).
• Preserved NoSuchTableError translation for genuine missing tables across both drivers.
• General syntax errors (SQLSTATE 42000) now correctly propagate their original exceptions rather than being masked as missing tables.
• Added regression tests in test/test_dialect_offline.py covering:
  • Negative cases for pycubrid syntax errors and C driver tuple args with native -493.
  • Positive cases for ANSI 42S02, Unknown class messages, and tuple args.
  • Exception propagation in both get_columns() and get_indexes().
• Added CHANGELOG entry under Fixed.

Verification

Ran test suite locally:

pytest test/test_dialect_offline.py

All 143 tests passed.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues remain in error-message matching and legacy driver handling.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Build missing-table detection messages from original errors only · New
• Document propagation of metadata-query exceptions · New

What changed in this PR

Fixes reflection error handling so native -493 syntax errors are not misreported as missing tables.

Changes:

• Refines missing-table detection.
• Adds regression tests for reflection errors.
• Updates the changelog.

File	Description
test/​test_dialect_offline.py	Adds syntax and missing-table regression tests.
sqlalchemy_cubrid/​dialect.py	Updates missing-table error classification.
CHANGELOG.md	Documents the reflection fix.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yeongseon]

Thanks a lot for tackling #454, @UsmanGhias. The predicate change is the right direction, and the new tests cover the pycubrid, wrapped and tuple-arg shapes well (they fail on main and pass with your change). A few things before we can merge:

1. CI lint fails on formatting. Please run ruff format sqlalchemy_cubrid test: dialect.py has extra blank lines around L128, and a few long asserts in test/test_dialect_offline.py need wrapping.
2. Please build the message from orig only and drop str(error) at dialect.py:117. The SQLAlchemy wrapper's string includes the SQL text, so a -494 permission error on a table whose name contains "Unknown class" now becomes NoSuchTableError, which main doesn't do. str(orig) already contains the tuple args, so the extra args loop can probably go too.
3. fix: do not translate every native -493 reflection error into NoSuchTableError #454 also asks for a docs update. Could you add a short note to the "Table Not Found During Reflection" section of docs/TROUBLESHOOTING.md (and docs/ko/TROUBLESHOOTING.md if you can), saying that syntax and other metadata-query errors now propagate as the original exception?

After you push, CI will re-run against current main. The earlier offline-test failures came from a missing greenlet on the old base, not from your change.

[UsmanGhias]

Thanks for the thorough review, @yeongseon.

I have addressed all the review items:

1. Ran ruff format using pinned ruff==0.16.8 to clean up the blank lines in sqlalchemy_cubrid/dialect.py and wrap the long assertions in test/test_dialect_offline.py.
2. Updated _is_no_such_table_error() to inspect str(orig) only, dropping str(error) and the orig.args loop. Added a regression test verifying that SQL text containing "Unknown class" on non-missing errors (such as a -494 permission failure) does not get translated into NoSuchTableError.
3. Added the note under "Table Not Found During Reflection" in both docs/TROUBLESHOOTING.md and docs/ko/TROUBLESHOOTING.md.
4. Rebased cleanly onto upstream main.
5. Regenerated docs/llms-full.txt via scripts/generate_llms_full.py to keep documentation sync checks passing.

All lint, format, typecheck, and offline tests pass locally.

Regards,
Usman
https://usmanghias.co.uk

[yeongseon]

Thanks for the quick and thorough update, @UsmanGhias. Formatting, the str(error) false positive, the args loop and the EN/KO troubleshooting note are all addressed, and CI is green. 🙏

Before merging I checked the predicate against a live server, and found something neither of us caught from the mocks. pycubrid currently reports every native -493 as sqlstate='42S02' with description='Table not found', including plain syntax errors. This is the same on CUBRID 10.2 and 11.4:

Statement	sqlstate	Server message
SHOW COLUMNS IN no_such_tbl	42S02	Syntax: Unknown class "no_such_tbl". show columns from …
SHOW INDEXES IN no_such_tbl	42S02	Syntax: Unknown class "no_such_tbl". show index from …
SELEC 1	42S02	Syntax: … Syntax error: unexpected 'SELEC' …
SHOW COLUMNS IN [bad name	42S02	Syntax: unterminated identifier

str(orig) also includes description='Table not found' for all of them. So with the recommended pycubrid driver, both the SQLSTATE check and the "Table not found" substring still classify syntax errors as missing tables, and #454 isn't fixed end to end. The _PycubridSyntaxError(sqlstate="42000") test double models a pycubrid that doesn't exist yet. That SQLSTATE mapping is a separate pycubrid bug, tracked in cubrid-lab/pycubrid#391.

The one signal that's reliable on both servers and both drivers is the server's own Unknown class "…" text. Could you:

1. Make _is_no_such_table_error() return True only when str(orig) contains Unknown class. Drop the sqlstate == "42S02" shortcut and the "Table not found" substring, since pycubrid attaches both to every -493 today.
2. Update the test doubles to match the real driver shape: a pycubrid-style syntax error with errno=-493, sqlstate="42S02" and a description='Table not found' suffix but no Unknown class must be False. Keep the Unknown class positives, including the CUBRIDdb tuple form.
3. Move the CHANGELOG entry from the released ## [1.7.1] section into ## [Unreleased] → ### Fixed, and use — to match the other entries.
4. Adjust the docs note (EN/KO, then regenerate docs/llms-full.txt) to say that NoSuchTableError is raised when the server reports Unknown class.

Missing-table reflection keeps working with that rule: both SHOW statements above carry Unknown class, so the existing #387 behaviour is preserved. Thanks again, this is a subtle one!

[yeongseon]

@codex review

[yeongseon]

Since the last round, #530 on main added _is_unknown_class_error() (the Unknown class-only rule from my previous comment) and moved most reflection call sites to it. Only get_columns() and get_indexes() still used the loose _is_no_such_table_error(), so as maintainer I narrowed this PR to those two sites and rebased it onto the 1.8.0 main:

• Both methods now use _is_unknown_class_error(), and _is_no_such_table_error() with its errno/SQLSTATE constants is removed.
• Offline tests: your predicate cases are kept but model the real pycubrid shape (errno=-493, sqlstate='42S02', description='Table not found', no Unknown class → not missing). A parametrized get_columns / get_indexes test checks that a syntax error propagates unchanged. The existing get_indexes missing-table test now uses the real Unknown class message.
• A live test in test/test_integration.py checks that a missing table still raises NoSuchTableError and that a -493 syntax error (a name containing ], rejected by CUBRID 11.4) propagates as the driver error. It fails on main and passes here. CUBRID 10.2 accepts ] in a quoted name, so that half runs on 11.4+ only.
• EN/KO troubleshooting note and llms-full.txt updated, and the CHANGELOG entry is under [Unreleased] → ### Fixed.

Verified with pycubrid 1.8.0 on CUBRID 10.2 and 11.4 (full test_integration.py), the full offline suite, and green CI. Approving.

[yeongseon]

Merged as a581fa7. Thank you @UsmanGhias! You found and pinned down #454, and your tests and troubleshooting note are what this change builds on. main had moved (with #530 and the 1.8.0 release), so I finished the last step on your branch: narrowing to the two remaining call sites and adding a live test. You are credited as co-author and in the CHANGELOG.

[UsmanGhias]

Welcome @yeongseon 🙌🏻