[el-marvin]

Fixes #753.

rel.create_view(name) and rel.to_view(name) took the whole string as one identifier, so a view meant for another
schema or database landed in the default schema under a dotted name:

import duckdb
con = duckdb.connect()
con.sql("CREATE SCHEMA s; ATTACH ':memory:' AS other")
rel = con.sql("SELECT 42 AS x")
rel.create_view("s.v")
rel.create_view("other.main.v")
print(con.sql("SELECT database_name, schema_name, view_name FROM duckdb_views() WHERE NOT internal").fetchall())
# [('memory', 'main', 'other.main.v'), ('memory', 'main', 's.v')]

rel.create("s.t") and rel.insert_into("s.t") have split a dotted name since #682, so the two view methods were
the odd ones out, and the only public way to put a view of a relation in another schema was
CREATE VIEW s.v AS {rel.sql_query()}, which loses what create_view gives: a view over a DataFrame stops working
once the relation is collected, and a relation with parameters cannot be written as text at all. A tuple of parts
already worked through create_view, because the C++ side builds a qualified name from it, but nothing said so and
the signature said str.

The name of a table or view is now read by one rule for create, to_table, insert_into, create_view,
to_view, con.table, con.view, duckdb.table and duckdb.view, and the signatures say str | tuple[str, ...].
A string goes to the engine's own reader of a qualified name string (QualifiedName::TryParse, the reader the
appender and SQL's pragma "show" already use), exposed through one seam entry: it is split at the dots outside
double quotes into catalog, schema path and name; a part in double quotes may hold dots, and a doubled quote inside
it is one quote; a trailing dot is dropped, as the appender has always read it; an empty part, an unterminated quote
or a quote in the wrong place is refused with the engine's own message. Every part is then one identifier taken
verbatim: no case folding, no trimming of whitespace, a keyword or a name with a space accepted quoted or not, and
a single quote or a backtick an ordinary character. These are the engine's rules for such a string, not the SQL
lexer's. A tuple of strings is the parts themselves, outermost first, each read by its text, so a str subclass
such as an Enum member counts by its value, in Python and in the C++ side's reading of a string or a tuple alike;
rel.query(name, sql) reads its name the same way. The view or table lands where CREATE VIEW <name>,
CREATE TABLE <name> or INSERT INTO <name> would put it: the default or an attached database, a schema, a nested
schema path of any depth, with USE in effect read as SQL reads it, and an unqualified name following the search
path; whether a view of a Python object may be stored is decided by the database the name resolves to, so a frame
view into an attached file database is refused and one into an attached in-memory database is created. con.table
and con.view check the connection first like the writers, read a tuple as the parts of the name, and keep their
two fall backs to one literal name for a string: when the reader refuses it, and when its first part names neither a
schema nor a database, which is what makes a file path such as people.csv work.

The limit of three parts that #682 put on create and insert_into goes. It existed because the client then handed
catalog, schema and name to the engine's Relation::CreateRel and InsertRel, which take only those three; since
the relational API moved onto statement text (7d1bcc7) the client renders the name into SQL, and the engine's
grammar, transformer and binder keep a schema path of any depth. So rel.create("memory.extra.s.t") now creates
the table instead of raising a NotImplementedException that pointed at SQL.

Found alongside: rel.create(("s", "t")), rel.insert_into(("s", "t")), con.table(("s", "t")) and
con.view(("s", "v")) concatenated the tuple's elements and wrote to, or read, a table called st in the default
schema, a silent wrong target; a tuple is now the parts of the name on the nine functions above (the appender still
takes a string only). An empty tuple, a non string part, or a name that is neither a string nor a tuple (None, an
int) raise InvalidInputException; an empty part in a tuple raises the reader's ParserException before anything
runs, so an open transaction stays usable, as it does for every string the reader refuses.

Behaviour changes, beyond the one the issue asks for:

• create_view and to_view read a dotted string as a qualified name. A name whose first part is neither a schema
  nor a database used to create a view named with its dots in the default schema and now raises the engine's
  CatalogException ("Schema with name a does not exist!"). With a schema x and an existing view main."x.y",
  create_view("x.y") used to replace that view and now creates x.y beside it; such a view is reached with the
  quoted name '"x.y"' or the one part tuple ("x.y",). A quoted name is read as SQL reads it:
  create_view('"a.b"') used to create a view whose name kept the quotes and now creates a.b.
• Two spellings that SQL rejects were silently merged by create, insert_into, con.table and con.view: a quote
  inside a bare part (a"b"c became the identifier abc) and anything but a dot after a closing quote ("a"b
  became ab). The writers now refuse them with the engine's messages; con.table and con.view read them as one
  literal name, their fall back for a file path, so a reader and a writer may read such a string differently.
  rel.query(name, 'pragma "show"(...)') described abc before and raises the same parser error as SQL's
  pragma "show" now.
• A trailing dot is dropped by every reader and writer: rel.create("s.t.") writes s.t where create and
  insert_into refused it, and con.table("a.") reads a where it read a table literally named a.. The
  messages for an unterminated quote and an empty quoted part are the engine's ("Unterminated quote in qualified
  name!", "Zero-length delimited identifier in qualified name!") instead of the client's earlier wording.
• A four or five part name on create, to_table or insert_into now runs; a missing schema inside an open
  transaction aborts it as the same SQL does, where the old refusal left it usable.
• A view that cannot hold its source (rows held in client memory, or a Python object in a database file) names the
  path with every part quoted, "s"."v" rather than "s.v".
• The Spark shim reads a table or view name with Spark's rules in createOrReplaceTempView, spark.table and
  saveAsTable alike: backticks quote a part, and double quotes are ordinary characters of the name. So
  spark.table("`my table`") reads the table my table as PySpark does, and a view made under `x.y` reads
  back with spark.table("`x.y`"); PySpark itself refuses a permanent table name with a dot
  (INVALID_SCHEMA_OR_RELATION_NAME, from SessionCatalog.validateName) or a backtick (INVALID_ATTRIBUTE_NAME_SYNTAX,
  from its name parser), which the shim keeps taking for saveAsTable as a leniency. The double quotes are
  a change for existing callers: spark.table('"orders"') read the table orders before and now raises
  CatalogException unless a table named with the quotes exists, and saveAsTable('"new"') created new and now
  creates a table named with the quotes; PySpark raises ParseException for both spellings. createOrReplaceTempView
  refuses a multi part name as PySpark does, with AnalysisException, error class
  TEMP_VIEW_NAME_TOO_MANY_NAME_PARTS and sqlState 428EK; a name Spark's reader refuses (x., a..b, the empty
  name) raises the shim's INVALID_ATTRIBUTE_NAME_SYNTAX in createOrReplaceTempView and saveAsTable, where
  saveAsTable("t.") created t. It used to make a temporary view named with the dots in the default schema that
  spark.table(name) could not read back.

The change is in duckdb/_relation.py (one name reader replaces _write_target; the string reader is the seam's;
con.table and con.view share the tuple reader; query() reads its name and its pragma "show" argument the
same way, the pragma's keywords matched in ASCII letters only with the whitespace between them left to the engine's
rules), one new entry in src/pyconnection/seam.cpp (_name_components over QualifiedName::TryParse; and
NameFromParts reads a str, whole or as a tuple's part, by its text), its stub, the _seam wrapper, and the
Spark shim's createOrReplaceTempView, SparkSession.table and saveAsTable with PySpark's error class. The
engine is untouched.

Tests, in tests/fast/relational_api/test_rapi_write_qualified_name.py, compare each relation call with the same
name in SQL on a second connection, and compare the full listing of views by database and quoted schema path, so a
view that landed under a literal dotted name fails the test: dotted and quoted names on create_view and to_view
in the default and an attached database, an ambiguous qualifier that is both a schema and a database, nested paths
of three to five parts on all five methods, replacing an existing nested view and refusing to without replace, the
tuple form with its errors (empty tuple, non string part, empty part, None and an int) and a str Enum part on
every method and on the seam itself, the refused spellings and the empty part on every method with the engine's
messages, the reader's verbatim rules by the stored spelling (trailing dot dropped, whitespace and case kept, a bare
keyword; string and tuple forms; the appender and insert_into agreeing on s.t.), a DataFrame relation and a
parameterized relation viewed under a dotted name after the relation is gone, a frame view into an attached file
database refused and into an attached in-memory database created, the held rows refusal, USE in effect, the
signatures, and the transaction staying usable after every client side refusal, string and tuple, on every method.
The tests that pinned the three part refusal are replaced by the tests that pin the landing.
tests/fast/api/test_table_name_resolution.py pins the tuple form of con.table, con.view, duckdb.table and
duckdb.view (never joined, with a table st beside s.t; never one literal name, with a table "a.b" and no
schema a), the closed connection refused before the name, the literal reading of the stray quote spellings and the
trailing dot; tests/fast/relational_api/test_rapi_query_binding.py pins pragma "show" against SQL on the stray
quotes, the trailing dot, a no-break or ideographic space between its words and a long s in the keyword, and a str
Enum name on query(); tests/fast/spark/test_spark_dataframe.py and test_spark_writer.py pin the Spark
refusals, the error class and sqlState, the read back of backtick quoted names through spark.table with a schema
of the same first part present, and, for the shim only, the dot and the backtick inside a quoted table name.

[el-marvin]

Changes requested.

I reviewed 1544acb, the one commit on top of the merge base 5b8e7e4; this is the first round. The head was built in Release by the review script; for the base I ran the merge base's Python sources over the same compiled extension, which is identical because the range changes no C++; and I ran the reporter's case on the duckdb 2.0.0 wheel. The premise holds: on 2.0.0 and on the base, rel.create_view("s.v") and rel.create_view("other.main.v") create views literally named s.v and other.main.v in memory.main; on the head they land in memory.s and other.main, and every one to five part name I tried on the five write methods lands where the same SQL puts it, with the engine's own errors, apart from the spellings the reader deliberately takes verbatim where SQL would not (spaces around a part, a bare keyword). The layer is right: the client already renders these names into statement text, so splitting the view name and lifting the three part limit belong here, not in the engine, and the description's account of why the limit existed checks out against the history. What must change before merge: a tuple with an empty part is not refused by the client, so it aborts an open transaction on the table writes; the name reader is a Python copy of the engine's qualified name reader beside the engine reader the package already calls, and the two disagree; con.table and con.view still join a tuple into one name, the same join this change fixes for the writes; and four rules the reader documents have no test. The evidence for each finding is in its inline comment.

Findings

1. A tuple with an empty part is not refused by the client, so create, to_table and insert_into send it to the parser and abort an open transaction, while the same mistake as a string, or on create_view, leaves the transaction usable (duckdb/_relation.py:3183).
Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.
Refuse an empty part of a tuple in _name_path with the string form's ParserException, and add the tuple form to test_refusal_keeps_the_transaction_usable.

2. _split_qualified is a Python copy of the engine's qualified name reader, beside the engine reader the package already calls for a table name in the appender, and the copy disagrees with it on a trailing dot and on the messages for an unterminated quote and an empty quoted part (duckdb/_relation.py:369).
Severity: should fix. Category: design. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.
Read a string name through one seam entry over QualifiedName::ParseComponents (or TryParse), called from _name_path, _name_parts and the pragma "show" branch of query(), and decide the trailing dot once, with a test.

3. con.table, con.view and duckdb.table still join a tuple's parts into one name and silently read another table, the join this change fixes for the writes; the fix sits in _name_path and not in _name_parts, the other caller of _split_qualified (duckdb/_relation.py:3177).
Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.
This holds the pull request because the change makes a tuple the documented form of a name and fixes the join at one of its two callers. Read a tuple as its parts in _name_parts, as the write methods now do, or refuse it, and test that con.table(("s", "t")) reads s.t.

4. Four rules the reader documents as its contract have no test: a trailing dot is refused, a part is not trimmed, case is not folded, a bare keyword is a name; reverting any one of them leaves all 490 touched tests green (tests/fast/relational_api/test_rapi_write_qualified_name.py:521).
Severity: should fix. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.
Add those spellings to the string and tuple cases of all five methods, asserting the spelling of the created entry, not only that a read finds it.

Behaviour changes to note

Not blocking; the issue asks for the first, and the maintainer decides how to announce it.

• create_view and to_view read a dotted string as a qualified name. A name whose first part is neither a schema nor a database used to create a view named with its dots in the default schema and now raises CatalogException ("Schema with name a does not exist!"). With a schema x and an existing view main."x.y", create_view("x.y") used to replace that view and now creates x.y beside it, leaving main."x.y" as it was. Such a view is reached with the quoted name '"x.y"' or the one part tuple ("x.y",).
• con.table and con.view still read a dotted string whose first part is neither a schema nor a database as one literal name, and now also a string the reader refuses, such as a"b"c; the writers refuse both. That is the readers' documented fall back for file paths, so a reader and a writer may read one string differently.
• A four or five part name on create, to_table or insert_into now runs; a missing schema inside an open transaction aborts it as the same SQL does, where the old refusal left it usable. The description says so.

Smaller things

• _name_text_sql (duckdb/_relation.py:3186) has the body of duckdb._expression.identifier, which takes the same tuple and returns the same text, and Table.name() renders a third copy; importing identifier removes one.
• _name_path's docstring says "a string split as SQL reads it", while the class notes say these are "the engine's rules for a qualified name string, not the SQL lexer's"; the class notes are right.
• The annotations say str | tuple[str, ...] and the message says "as a tuple of its parts", but a list is accepted too (rel.insert_into(["s", "t"]) writes s.t); annotate what is accepted, or refuse a list.
• A part that is a str subclass is read by its value in create and insert_into but by __str__ in create_view, because NameFromParts calls nb::str(part): with class Names(str, enum.Enum), create((Names.SCHEMA, Names.TABLE)) writes s.t while create_view((Names.SCHEMA, Names.VIEW)) raises "Schema with name Names.SCHEMA does not exist!". Returning plain strings from _name_path (str.__str__(part)) gives one reading.
• In test_name_with_a_stray_quote_reads_the_literal_name (tests/fast/api/test_table_name_resolution.py) the second with connect(STRAY_QUOTES) block uses neither parameter, so its con.view('v"w') check runs three times unchanged; it reads better as a test of its own.
• Six refusal tests in test_rapi_write_qualified_name.py (lines 482, 490, 500, 508, 536 and 622) open a second connect(FLAT_SETUP) only to read a table count that never changes; taking before = table_count(con) ahead of the call, as assert_refused_as_sql_refuses does, saves a database setup per case.
• The Spark shim keeps a dotted temporary view name as one name, so with a schema x present spark.table("x.y") cannot read back what createOrReplaceTempView("x.y") made ("Table with name y does not exist!", base and head alike). The new test's skip reason says PySpark refuses such a name; refusing it here too would keep the two halves of the shim consistent. This predates the change.

Questions

• The description's statement that PySpark refuses a multi part temporary view name was not run here (no Spark on this machine); the skip in test_temp_view_with_a_dotted_name_is_one_name rests on it.

Outside findings not taken

• A regular expression form of _split_qualified: style, and the second finding removes the copy.
• Moving the empty part refusal into _split_qualified: where that rule lives follows from the first two findings.
• The tuple checks being in both _name_path and the C++ NameFromParts, the C++ copy unreachable from create_view: the C++ copy still serves _table_kind, and the two agree word for word.
• Duplicated helpers and parameter lists in the new test file: style.
• The second _ensure_open in the pragma "show" branch and the character at a time splitter: a few microseconds against a write of about 290 microseconds, on no hot path.
• Two renderers of a quoted path, the Python quote and the C++ QuotedName: they agree on every name probed.
• _SHOW_PRAGMA matching ſhow without re.ASCII, and a lone surrogate in a name escaping as a nanobind TypeError: adversarial input, and the base does the same.
• Sharing _expression.name_parts as the tuple check: it raises TypeError and ValueError for expression names, another contract; taken only as evidence for the first finding.
• A string con.table reads literally that the writers refuse, such as a"b"c: the readers' documented fall back for file paths; listed under behaviour changes.
• Refusals of four part names inside BEGIN now aborting the transaction: as SQL does, and the description says so.

What I verified

• Head 1544acb against merge base 5b8e7e4, one commit, seven files; no C++, stub, CMake or pyproject change in the range.
• Builds: the head's Release build (BUILD OK); the base as the merge base's Python sources over the head's compiled extension; the release as the duckdb 2.0.0 wheel.
• The reporter's script on 2.0.0, the base and the head.
• Probes, each connection at memory_limit 8GB and 8 threads: create_view against CREATE OR REPLACE VIEW of the same text for 49 names (one to five parts, attached databases, quoted dots and doubled quotes, a missing catalog or schema, the temp catalog, a table in the way, empty, dots only, leading and trailing dots, stray and unterminated quotes, case, spaces, keywords); the five methods with four and five parts; tuples, lists, None, an int, bytes and empty parts; each refusal inside an open transaction against the same SQL; a closed connection; a Python object into file and memory databases by path; held rows; replace on one to four part names; USE; con.table, con.view, con.appender and pragma "show" against the engine's readers; the Spark shim; a str Enum part; the reader's cost.
• Tests: the four touched files on the head, 490 passed in 8.26s; the head's tests against the base's sources, 125 failed, 365 passed. In a clone of the head I reverted each changed area in turn and ran the four files: the old splitter, its check for a quote inside a bare part, its check for text after a closing quote, its doubled quote rule, the string empty part check, the type check, the empty tuple check, the non string part check, create_view passing the path, a three part limit on the table writes and on the views, and the Spark line each turn tests red; dropping _ensure_open turns tests/fast/relational_api/test_rapi_close.py red; the pragma "show" rewrite is a refactor and stays green; the rules of the fourth finding stay green. tests/fast/relational_api, tests/fast/api and tests/fast/spark on the head: 5242 passed, 42 skipped, 13 xfailed in 105.00s.
• Outside reviews: the code-review skill at high effort (nine finder reports: simplification, altitude, reuse, efficiency, line scan, wrapper and proxy, language pitfalls, removed behaviour, cross file) and Codex at xhigh, read only. Every finding taken was reproduced here.
• CI at the time of review: every finished check green (Linux and Windows seeds and sanity jobs, the sdist, the full API build with assertions, code quality, submodule sanity); the two macOS seed jobs were still pending.

What held

• The reporter's case, and every one to five part name I tried on all five methods (other than the verbatim spellings above), in the default and attached databases and in nested schemas, with quoted parts holding dots and doubled quotes, land where SQL puts them, also with USE in effect.
• A missing catalog or schema, an ambiguous qualifier, the temp catalog, a table where the view should go, and replace=False on an existing view give the engine's errors, with SQL's kind and text.
• A view over a DataFrame or with parameters reads after the relation is gone; a Python object is refused for a file database and allowed for an attached in-memory one, decided by the path's catalog; held rows are refused, naming the quoted path.
• Every string refusal happens before anything runs and leaves an open transaction usable; a closed connection is refused first; None, an int and bytes are refused with InvalidInputException.
• The three stray quote spellings now match the engine's reader, in con.table and in pragma "show".
• The reader costs about 2.6 microseconds per name against about 290 microseconds for a create_view; no hot path is touched.

[el-marvin]

A tuple with an empty part is not refused here, so create, to_table and insert_into send it to the parser and abort an open transaction, while the same mistake as a string, or on create_view, leaves the transaction usable.

Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.

    if not all(isinstance(part, str) for part in name):
        raise _error(_duckdb.InvalidInputException, "Every part of a name must be a string")
    return tuple(name)

The string branch above refuses an empty part before anything runs. The tuple branch returns it, create renders CREATE TABLE "s"."" AS ..., and the parser refuses that after the statement was submitted, which invalidates the transaction. create_view parses its own CREATE VIEW head in DuckDBPySeam::ViewStatement before it runs anything, so there the transaction survives. Inside BEGIN; CREATE TABLE sentinel(i INT), then the call, then INSERT INTO sentinel VALUES (1), on the head:

rel.create       ('s', '')  ParserException: Parser Error: zero-length delimited identifier | after: TransactionException: TransactionContext Error: Current transaction is aborted (please ROLLBACK)
rel.insert_into  ('s', '')  ParserException: Parser Error: zero-length delimited identifier | after: TransactionException: TransactionContext Error: Current transaction is aborted (please ROLLBACK)
rel.create_view  ('s', '')  ParserException: Parser Error: zero-length delimited identifier | after: ok [(1,)]
rel.create       's.""'     ParserException: Parser Error: Zero-length identifier in qualified name! (input: s."") | after: ok [(1,)]

('',) and ('', 't') behave the same. The base had no tuple form on the table writes (it joined ("s", "t") into st), so this is new here. test_empty_part_refused (line 506) runs outside a transaction and matches the parser's message, so it passes either way. The package already holds this rule for tuple names elsewhere: duckdb._expression.name_parts refuses an empty part.

Ask: refuse an empty part of a tuple here with the same ParserException as the string form, and add the tuple form to test_refusal_keeps_the_transaction_usable.

[el-marvin]

_split_qualified is a Python copy of the engine's qualified name reader, beside the engine reader the package already calls for a table name, and the copy disagrees with it on a trailing dot and on the messages for an unterminated quote and an empty quoted part.

Severity: should fix. Category: design. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.

The engine's reader is TryParseComponents in external/duckdb/src/parser/qualified_name.cpp, behind QualifiedName::ParseComponents and QualifiedName::TryParse. The appender already reads its table name with it, in src/pyappender.cpp:

static ResolvedTable ResolveTable(ClientContext &context, const string &text) {
	QualifiedName parsed;
	string parse_error;
	if (!QualifiedName::TryParse(text, parsed, parse_error)) {
		throw ParserException(parse_error);
	}

and _column_name in this file reads a column reference through the engine with _seam.column_reference. On the same strings, on the head (query_table and SQL's pragma "show" are the engine's readers of a name string):

con.appender("s.a.")                         opens the appender on s.a
rel.insert_into("s.a.")                      ParserException: Zero-length identifier in qualified name! (input: s.a.)
con.sql("pragma \"show\"('s.')")             CatalogException: Table with name s does not exist!   (it read s)
rel.query("v", "pragma \"show\"('s.')")      ParserException: Zero-length identifier in qualified name! (input: s.)
query_table('"'), pragma "show"('"')         Unterminated quote in qualified name! (input: ")
rel.create('"'), rel.query(pragma "show")    Unterminated quoted identifier in qualified name! (input: ")
query_table('s.""'), pragma "show"('s.""')   Zero-length delimited identifier in qualified name! (input: s."")
rel.create('s.""'), rel.query(pragma "show") Zero-length identifier in qualified name! (input: s."")

The base already had a Python splitter, but this change rewrites it whole, makes it the reader of create_view and to_view, and documents it as the engine's rules with "the one difference" a trailing dot; the two message differences are not mentioned, and the pragma "show" branch of query(), which exists to answer as SQL's pragma does, still refuses s. and words the other two unlike SQL. The new parity tests cover only the two messages the copy took over.

Ask: read a string name through one seam entry over the engine's reader that returns the parts and raises the engine's ParserException, and call it from _name_path, _name_parts (keeping its fall back to one literal name) and the pragma "show" branch. If the writes must still refuse a trailing dot the engine drops, make that one explicit check in _name_path, say so in the class notes, and test it. The pins on the old wording (line 535 of the new test file, and the '""' and 's.""' cases at line 521) then take the engine's.

[el-marvin]

con.table, con.view and duckdb.table still join a tuple's parts into one name and silently read another table: the join this change fixes for the writes, fixed here in _name_path but not in _name_parts, the other caller of _split_qualified.

Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.

This holds the pull request: the description fixes rel.create(("s", "t")) writing to st as a silent wrong target and makes a tuple the documented form of a name, but the helper that joins is still reached with a tuple from the readers, at line 3530:

def _name_parts(name: str) -> tuple[str, ...]:
    """The parts of a dotted name, or the whole name as one part when it cannot be one (a path such as ./x.csv)."""
    try:
        parts = tuple(_split_qualified(name))

_split_qualified walks whatever sequence it is given, so the elements of a tuple become the characters of one part. With tables s.t and st and views s.v and sv, on the head and on the base alike:

con.table(("s", "t"))                     [('wrong st',)]
con.table(["s", "t"])                     [('wrong st',)]
con.view(("s", "v"))                      [('wrong sv',)]
duckdb.table(("s", "t"), connection=con)  [('wrong st',)]

So rel.create(("s", "t")) followed by con.table(("s", "t")) reads a different table without an error. The stubs annotate table_name: str, but nothing refuses a tuple at run time.

Ask: give _name_parts the type step this function has: a tuple is the parts of the name, as on the five write methods (or refuse it, if con.table should take strings only), and anything else is refused; never join. Put the check where both callers pass through (the shared reader, or the seam entry the finding on _split_qualified asks for), and add a test that con.table(("s", "t")) reads s.t or raises.

[el-marvin]

Four rules the reader documents as its contract have no test: a trailing dot is refused, a part is not trimmed, case is not folded, and a bare keyword is a name; reverting any one of them leaves every touched test green.

Severity: should fix. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.

The class notes say "Every part is then one identifier taken verbatim: no case folding, no trimming of whitespace, a keyword or a name with a space accepted quoted or not", and the docstring of _split_qualified makes the trailing dot "the one difference" from the engine. In a clone of the head I changed _split_qualified one way at a time and ran the four touched test files (490 tests):

drop a trailing dot, as the engine does   490 passed
strip whitespace from each part           490 passed
lower case each part                      490 passed

Every name in the new tests is lower case without leading or trailing spaces and none ends in a dot, and a read that succeeds cannot show the spelling, because the catalog compares names without regard to case. The head does follow the rules (create_view("s. v") makes memory.s." v", create_view("S.V") makes memory.s."V", create_view("select") makes memory.main."select"), so this is coverage, not behaviour; at the base the trailing dot refusal on create and insert_into was untested too.

Ask: add "s.", "s. v", "S.V" and "select" (and the tuple forms where they apply) to the cases of all five methods, asserting the created entry's spelling from duckdb_views() or duckdb_tables(), so that changing a rule, including the change the finding on _split_qualified asks for, has to change a test.

[el-marvin]

Changes requested.

I reviewed a7a998a, one commit on top of 1544acb, which the first round reviewed; it answers that round's four requests and adds C++: a seam entry over the engine's qualified name reader and a change to how the seam reads a str subclass. The head was built in Release by the review script. Because the range now holds C++, this round I built the merge base 5b8e7e4 in Release the same way, instead of running the base's Python over the head's extension as the first round did, and reran every first round probe on the 2.0.0 wheel, the base build and the head. The premise still holds: on 2.0.0 and the base rel.create_view("s.v") makes memory.main."s.v", on the head memory.s.v. All four first round findings are addressed, each checked by rerunning its probe and by reverting the fix in a clone of the head, which turns the new tests red. The layer is right, and better than the first round's version: a name string now goes through QualifiedName::TryParse, the reader the appender and SQL's pragma "show" use, so the writers, the appender and query()'s pragma "show" branch agree with SQL on every spelling I tried, messages included, and the readers do too, apart from their documented fall back to one literal name. Two things must change before merge: createOrReplaceTempView in the Spark shim now unquotes a backtick-quoted name, but the shim's spark.table still reads the same string by DuckDB's rules, so a view made under a backtick-quoted name cannot be read back with spark.table, which worked on the base; and no test pins that a view of a Python object is refused or allowed by the database its dotted name names, a rule the change's main feature now reaches from every dotted string (I missed this in the first round; it was already in the range). One nit remains on the pragma "show" pattern. The evidence for each is in its inline comment.

Since the last review

• 1, a tuple with an empty part aborting an open transaction: addressed. Inside BEGIN, ("s", ""), ("",) and ("", "t") on all five write methods raise ParserException: Parser Error: Zero-length identifier in qualified name! (input: "s"."") (and the two other inputs) before anything runs, and the sentinel insert after each one succeeds (15 of 15). con.table and con.view refuse the same tuples in an open transaction that stays usable. Removing the check turns 22 tests red.
• 2, a Python copy of the engine's reader: addressed. _split_qualified is gone; _name_path, _name_parts and the pragma "show" branch read a string through _duckdb._name_components, which calls QualifiedName::TryParse and raises its ParserException unchanged. On "", s."", ", s."v, s., s..t, a"b"c, "a"b, s."a"b, "s.t" and s.ab, rel.query(name, 'pragma "show"(...)') now gives exactly what SQL's pragma "show" gives. The trailing dot is decided once: dropped, as the engine reads it; con.appender and insert_into agree on a., s.t., "a, "s, s."", s..t and the stray quote spellings. Refusing the trailing dot again in the writers turns 12 tests red, keeping it as a literal name in the readers turns one red.
• 3, con.table and con.view joining a tuple: addressed. With tables s.t and st and views s.v and sv, con.table(("s", "t")), con.table(["s", "t"]), con.view(("s", "v")) and duckdb.table(("s", "t"), connection=con) read s.t and s.v. Joining the tuple again turns 11 tests red.
• 4, four reader rules without a test: addressed. TestReaderRules asserts the stored spelling; in a clone of the head, refusing a trailing dot turns 12 tests red, trimming each part 10, lower casing each part 8.
• Smaller things from the first round: _name_text_sql is gone in favour of _expression.identifier; _name_path's docstring is corrected; a str subclass part is read by its value in Python and in the seam (create_view((Names.SCHEMA, Names.VIEW)) now makes s.v; reverting the C++ half in a rebuilt clone turns the five test_str_subclass_part_is_read_by_its_value cases red, reverting the Python half turns the same five red); the repeated test block and the second connections are gone. A list as a name: addressed by decision; the public contract is str | tuple[str, ...] and a list keeps working undocumented, because create_view took one on 2.0.0 through the seam; that reason holds. The Spark shim's dotted temporary view name: now refused as PySpark refuses it; see the finding below for what that change left behind.
• The first round's question, whether PySpark refuses a multi part temporary view name: settled. Spark's Dataset.createTempView raises TEMP_VIEW_NAME_TOO_MANY_NAME_PARTS with the name as given, sqlState 428EK in error-conditions.json; the shim's error class, message and sqlState match.

Findings

1. A temporary view name in backticks is unquoted by createOrReplaceTempView but not by SparkSession.table, so spark.table no longer finds a view the shim just made under that name, where the base found it (duckdb/experimental/spark/sql/dataframe.py:748).
Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.
Read the name in SparkSession.table with the same _name_parts and pass its parts to conn.table, and read the view back with spark.table in the new test.

2. No test pins that a view of a Python object is refused or allowed by the database its qualified name names; the check now decides it for every dotted string given to create_view, and making it look at the default database alone leaves every touched test green (tests/fast/relational_api/test_rapi_write_qualified_name.py:610, the check at src/pyconnection/seam.cpp:630).
Severity: should fix. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.
Add a frame view into an attached file database by a dotted string and a tuple (refused, quoted path, no view left) and from a file backed connection into an attached ':memory:' database (created, readable after the frame is gone).

3. re.ASCII on _SHOW_PRAGMA narrows \s too, so a pragma "show" with a no-break or ideographic space between the words is no longer rewritten and rel.query describes the catalog entry instead of the relation, unlike the base (duckdb/_relation.py:77).
Severity: minor. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.
Drop the flag, or confine it to the keyword with (?a:show); a nit.

Behaviour changes to note

Not blocking; the maintainer decides how to announce them. The description lists each one.

• create_view and to_view read a dotted string as a qualified name; a first part that is neither a schema nor a database now raises CatalogException, and create_view('"a.b"') makes a.b where it kept the quotes.
• con.table and con.view still read a string the engine's reader refuses (a"b"c, "a"b, "s, ./x.csv) as one literal name, which the writers and the appender refuse; that is the readers' fall back for file paths.
• A trailing dot is dropped by every reader and writer: with a table literally named a., con.table("a.") now reads a.
• A four or five part name on create, to_table or insert_into runs, and a missing schema in an open transaction aborts it, as the same SQL does.
• The Spark shim refuses a multi part temporary view name with PySpark's error class and unquotes backticks; a name its reader refuses (x., a..b, the empty name) raises the shim's INVALID_ATTRIBUTE_NAME_SYNTAX, where PySpark raises its own "Invalid view name" class.

Smaller things

• The tuple form skips the readers' fall back to a literal name (isinstance(name, str) in _catalog_relation, duckdb/_relation.py:3550), but no test has a tuple whose first part is neither a schema nor a database; with the check removed, con.table(("x", "y")) beside a table "x.y" fails with AttributeError: 'tuple' object has no attribute 'replace' and all touched tests stay green. A case in TestTupleNames (a table "x.y", no schema x, the tuple raising CatalogException) would pin the rule.
• The empty name check in createOrReplaceTempView (if not parts[0]:, dataframe.py:753) has no test: removing it leaves every touched test green, and createOrReplaceTempView("") then raises the relation API's ParserException instead of the INVALID_ATTRIBUTE_NAME_SYNTAX the description names.
• test_temp_view_with_a_dotted_name_is_refused checks the error class but not the sqlState 428EK the change adds to error_classes.py.
• DataFrameWriter.saveAsTable (duckdb/experimental/spark/sql/readwriter.py:393) passes its Spark name to relation.create as DuckDB reads it: saveAsTable("`x.y`") looks for a schema named `x (base and head alike), and saveAsTable("t."), which Spark's parser refuses and the base refused, now creates t. Reading the name with _name_parts there too, with the fix of the first finding, would give the shim one rule for its names.
• con.table and con.view are the functions _connection_table and _connection_view (duckdb/_relation.py:4358 and 4363), still annotated table_name: str and view_name: str, so inspect.signature(con.table) shows (table_name: 'str') while the stubs and duckdb.table say str | tuple[str, ...].
• The class notes say a string the reader refuses, "such as a file path, is one name"; but the reader takes people.csv as ['people', 'csv'], and the file is read because people names no schema or database (with a schema people holding a table csv, con.table("people.csv") reads the table, base and head alike). The notes would say what holds by naming that second fall back, which _catalog_relation's docstring already states.
• The module docstring of test_rapi_write_qualified_name.py says the writes "land where SQL puts the same name, and refuse a name as SQL does", which TestReaderRules in the same file contradicts on purpose (s. new, a trailing dot and a bare keyword are taken where SQL reads or refuses them differently).
• The description's "a tuple is now the parts of the name everywhere" reads as if con.appender took one too; it takes a string only (TypeError on a tuple, as before). Naming the nine functions instead of "everywhere" says what holds.

Outside findings not taken

• Whitespace around a temporary view name (" v " keeps its spaces, "`v` " is refused, where Spark's parser makes both v): adversarial input; the shim also accepts names Spark refuses ("q", my view), which is leniency, not a wrong target.
• No test with a list as a name: a list is accepted undocumented by the author's decision, which this review accepts; it is not a contract to pin.
• The tuple checks in _name_tuple duplicating NameFromParts in C++: no public method hands the C++ reader an unchecked tuple (create_view passes the parts _name_path returned, table_kind gets a checked step's parts or the query name), the same reason as in the first round.
• A str subclass becoming the alias when con.table reads a string whole: the base does the same, and it needs a str Enum whose value is a dotted name with no schema.
• An embedded NUL and a lone surrogate in a name: the seam keeps a NUL (length aware on both sides), and a lone surrogate fails nanobind's conversion with a TypeError before the seam runs, on writers and readers alike; adversarial input, and the base fails on it too.

What I verified

• Head a7a998a against merge base 5b8e7e4, two commits, ten files, 791 lines added and 162 removed; the new commit 1544acb..a7a998a changes 352 and 104. The commit holds what the author's reply lists and nothing beyond it.
• Builds: the head's Release build (BUILD OK); a Release build of the merge base with the same two uv sync steps, Python 3.13; the duckdb 2.0.0 wheel. Probes run each build without an import time rebuild.
• Every first round probe rerun on all three builds: the reporter's case, 49 names on create_view against CREATE OR REPLACE VIEW, the readers against query_table, pragma "show" and pragma_table_info, four and five part writes, the tuple refusals inside BEGIN, storage, replace and USE, the messages, the siblings and the appender, the Spark shim, the str Enum parts.
• The first round's outside review probes (42 scripts, copied to the round folder) on base and head: the differences are the intended behaviour changes; two scripts import helpers this commit removes (_split_qualified, _name_text_sql), and the fuzz of the removed Python reader and two timing scripts of the test setup were not rerun.
• New probes: _duckdb._name_components on 26 inputs (empty, dots only, leading, trailing and doubled dots, empty and unterminated quotes, stray quotes, quoted dots and doubled quotes, NUL, lone surrogates, non ASCII, spaces, case, a keyword, six parts, a 10,000 character part, %s in the input) against the engine's query_table: same messages and exception class (duckdb.ParserException), a NUL kept, a str Enum and a str subclass with its own __str__ read by value, None, an int, bytes and a tuple refused by nanobind with TypeError. The appender, insert_into, con.table and query_table on 14 strings and three tuples. The Spark shim's create and read back on 14 names, on base, head, and a clone with SparkSession.table reading Spark names.
• Limited API: src/pyconnection/seam.cpp compiled with the build's own command from compile_commands.json plus -DPy_LIMITED_API=0x030C0000 -fsyntax-only, head and base: exit 0, no errors, 26 warnings each.
• Tests: the four touched files on the head, 586 passed in 12.26s; the same files on the base build, 196 failed, 390 passed; every test function this round adds fails there in at least one case, except test_view_with_a_stray_quote_reads_the_literal_name, which pins a reading the base already had. Reverts in a clone of the head, each run against the touched files plus test_rapi_close.py and test_seam.py (962 tests): the writers refusing a trailing dot (12 failed), the readers keeping it (1), trimming (10), lower casing (8), no tuple empty part check (22), tuple parts not cast to str (5), the reader checking the name before the connection (4), the tuple joined in the readers (11), query() not casting its name (1), pragma "show" refusing a trailing dot (1), the Spark multi part refusal removed (1), the Spark view made from the raw name (1), the C++ NameFromParts change reverted in a rebuilt clone (5 of 960); green: the tuple skipping the readers' fall back, the Spark empty name check, and re.ASCII on the pragma "show" pattern (adversarial input only).
• Full tests/fast on the head, eight workers and a 600 second timeout per test: 16999 passed, 83 skipped, 223 xfailed in 868.26s, the figure the author reports.
• The two new findings' probes on base and head (pragma "show" with an ASCII, a no-break and an ideographic space; saveAsTable on three names; frame views into attached file and memory databases from memory and file connections), and the storage rule's revert in a rebuilt clone of the head (963 passed with the check looking at the default database only).
• Outside reviews: Codex at xhigh, read only, and the code-review skill, which ran at max effort rather than the high it was given, because it did not read the level from its arguments. Every finding taken was reproduced here.
• CI on a7a998a when this was written: every finished check passed (submodule sanity, code quality, the sdist, the full API build with assertions, the seed jobs on Windows amd64 and ARM64, Linux x86_64 and aarch64 and macOS x86_64, and both cp311 Linux sanity jobs); the macOS arm64 seed job was still queued. Nothing is red.

What held

• The reporter's case, and every one to five part name of the first round's matrix on all five write methods, land where SQL puts them, with the engine's errors.
• Every string the engine's reader refuses is refused by every writer before anything runs, with the engine's message, and leaves an open transaction usable; so is every bad tuple.
• The writers, the appender and pragma "show" read every string I tried the same way as SQL; the readers differ only on strings the engine refuses, as documented.
• A tuple is read as its parts on all nine functions and is never joined; a str subclass counts by its value in Python and in C++, also in query().
• The new seam entry passes the engine's exception and message through unchanged, owns its result strings, keeps a NUL, runs under the GIL for a parse that needs no release, and compiles under the limited API.
• con.table and con.view refuse a closed connection before reading the name.
• The Spark shim's refusal of a multi part temporary view name matches PySpark's error class, message and sqlState.
• No hot path is touched: the readers run once per con.table call and the writes once per statement.

[el-marvin]

A temporary view name in backticks is now unquoted on the way in but not on the way out: createOrReplaceTempView reads it with the shim's Spark name reader, while SparkSession.table still passes the same string to con.table, which reads it by DuckDB's rules, so spark.table no longer finds a view the shim just made under that name; on the base it did.

Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.

The new reading here:

        parts = _name_parts(name)
        ...
        relation.create_view((parts[0],), True)

and the shim's reader of the same name, unchanged, in duckdb/experimental/spark/sql/session.py:297:

    def table(self, tableName: str) -> DataFrame:  # noqa: D102
        relation = self.conn.table(tableName)
        return DataFrame(relation, self)

df.createOrReplaceTempView(name) followed by spark.table(name), each connection at 8 threads and an 8GB memory limit:

name        base 5b8e7e4                              head a7a998a
`v`         makes `v` (backticks kept), reads it      makes v, CatalogException: Table with name `v` does not exist!
`x.y`       makes `x.y`, reads it                     makes x.y, CatalogException: Table with name `x.y` does not exist!
`a``b`      makes `a``b`, reads it                    makes a`b, CatalogException: Table with name `a``b` does not exist!
`select`    makes `select`, reads it                  makes select, CatalogException: Table with name `select` does not exist!

The base was wrong the other way (it kept the backticks in the view's name, so spark.sql("SELECT a FROM `x.y`") missed it), but its two halves agreed. In PySpark both halves read the string with the same SQL identifier parser (Dataset.createTempView calls parseMultipartIdentifier, DataFrameReader.table calls parseTemporalTableIdentifier), so spark.table("`x.y`") reads the view x.y there, and backticks are how a Spark user names a temporary view with a dot, a space or a hyphen. test_temp_view_name_is_read_with_spark_quoting reads the view back only through spark.sql, whose text DuckDB's parser unquotes, so it passes either way.

In a clone of the head I changed SparkSession.table to self.conn.table(_name_parts(tableName)) (the connection's table() takes a tuple since this change): every name above, and every other name createOrReplaceTempView accepts in the probe, reads back with spark.table, and tests/fast/spark passes (1828 passed, 7 skipped, 13 xfailed).

Ask: read the name in SparkSession.table with the same _name_parts and pass its parts to conn.table, and read the view back with spark.table in test_temp_view_name_is_read_with_spark_quoting.

[el-marvin]

No test pins that a view of a Python object is refused or allowed by the database its qualified name names: the check in src/pyconnection/seam.cpp:630 now decides this for every dotted string given to create_view, and making it ignore the named database leaves every touched test green.

Severity: should fix. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: widened.

The check, unchanged, which this change makes reachable from a dotted string (on the base create_view("f.v") made main."f.v" in the default database, so only a tuple reached it):

	if (!temporary && inliner.HoldsProcessLocalInput() && StoredInAFile(context, name)) {

The only test of the refusal is tests/fast/api/test_seam.py::test_a_python_object_cannot_be_stored_in_a_file, which calls the private _create_view("p", ...) with an unqualified name on a file database. TestViewOwnsItsSources covers a frame view into db2, which is ':memory:'. On the head, a relation over a DataFrame:

memory default, file database f attached:  create_view("f.v")          refused: Cannot create the view "f"."v": it reads a Python object ...
                                           create_view(("f","main","v2")) refused, the same
                                           create_view("v3")           created
file default, ':memory:' attached as m:    create_view("m.v")          created
                                           create_view(("m","main","v2")) created
                                           create_view("v3")           refused

That is right. In a clone of the head I changed the call to StoredInAFile(context, QualifiedName(name.Name())), so the check looks only at the default database, and rebuilt: the touched test files and test_seam.py and test_relation_to_view.py all pass (963 passed), while create_view("f.v") from a memory connection now fails at commit with TransactionException: Failed to commit: Cannot serialize a table and create_view("m.v") from a file connection is refused. The description also names this refusal among the messages that now quote every part of the path, which no test checks either.

Ask: add to TestViewOwnsItsSources a frame view into an attached file database by a dotted string and by a tuple, refused with the quoted path and leaving no view, and a frame view from a file backed connection into an attached ':memory:' database, created and readable after the frame is gone.

[el-marvin]

re.ASCII also narrows \s to ASCII whitespace, so a pragma "show" written with a no-break or ideographic space, which the engine's parser reads as a space, is no longer rewritten and rel.query(name, ...) describes the catalog entry instead of the relation, as it did on the base.

Severity: minor. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.

With a table t(o VARCHAR) and con.sql("select 1 as a").query("t", text):

text                         base                      head
pragma "show"('t')           [('a', 'INTEGER', ...)]   [('a', 'INTEGER', ...)]
pragma\u00a0"show"('t')      [('a', 'INTEGER', ...)]   [('o', 'VARCHAR', ...)]
pragma\u3000"show"('t')      [('a', 'INTEGER', ...)]   [('o', 'VARCHAR', ...)]

The flag was added for ſhow, which the first round left out as adversarial input; this trades that corner for another and has no test either way. Dropping the flag restores the base's reading; (?a:show) around the keyword alone would keep both. A nit; take it or leave it.

[el-marvin]

Changes requested.

I reviewed 1d10a6e, one commit on top of the last round's head a7a998a, Python and tests only: eight files, 78 lines added and 11 removed, no C++ in the diff of a7a998a..1d10a6e. I ran it on a Release build of the head, on the Release build of the merge base 5b8e7e4 from the last round, and on the released duckdb 2.0.0, and I rebuilt a clone of the head with the last round's mutation of the file database check. The premise still holds: 2.0.0 and the merge base put create_view("s.v") in the default schema under the dotted name, the head puts it in schema s. The approach sits at the right layer, and the three items of the last round are fixed at their root: spark.table and saveAsTable now read a name with the shim's own Spark reader and hand the parts to the tuple form, the file database check is pinned in both directions with a string and a tuple, and the pragma "show" pattern keeps Unicode whitespace with ASCII keywords. One thing must change before merging: the new saveAsTable test fails when the suite runs against PySpark, because PySpark refuses the table names it creates. A minor coverage gap and three smaller things follow.

Since the last review

• spark.table could not read back a backtick quoted temporary view: addressed. The last round's probe on the head: `v`, `x.y`, `ab``, ``select`` and'"q"'all read back throughspark.table, where the last head raised CatalogException. `` saveAsTable("x.y") `` makes main."x.y"and reads back, andsaveAsTable("t1.")raisesINVALID_ATTRIBUTE_NAME_SYNTAXagain where the last head createdt1. Reverting SparkSession.tableto the raw string fails two tests, revertingsaveAsTable` fails one.
• No test pinned that a view of a Python object follows the database its name names: addressed. With StoredInAFile(context, QualifiedName(name.Name())) at src/pyconnection/seam.cpp:630 in a rebuilt clone, all four cases of test_frame_into_an_attached_file_database_refused and test_frame_into_an_attached_memory_database_from_a_file fail (4 failed, 904 passed) and they pass on the head. On the merge base the two string cases fail and the two tuple cases pass, as expected: the C++ side already read a tuple as parts.
• re.ASCII narrowed the whitespace of the pragma pattern: addressed. pragma and "show"('t') separated by a no-break space (U+00A0) or an ideographic space (U+3000) in rel.query describe the relation again, as 2.0.0 does; pragma ſhow('t') and pragma "ſhow"('t') now raise the engine's "Pragma Function with name ſhow does not exist!" exactly as SQL does, where 2.0.0 and the base rewrote them. The last round's pattern fails the new test. Using (?a:...) around the keywords alone is the right shape.
• The last round's smaller things: all taken and verified. The tuple that skips the literal fall back is pinned (removing the isinstance check at duckdb/_relation.py:3553 fails test_parts_are_not_one_literal_name), the empty temporary view name is pinned (removing the check at dataframe.py:753 fails test_temp_view_with_a_dotted_name_is_refused), sqlState 428EK is asserted, saveAsTable reads Spark names, _connection_table and _connection_view carry str | tuple[str, ...] (the runtime signature shows it), the class notes and the test module docstring are corrected, and the description names the nine functions.
• The four findings of the first round stay addressed: its probes give the same output on the head as on the last head, apart from object addresses and file paths in tracebacks.

Findings

1. The new saveAsTable test fails against PySpark, which refuses the table names it creates (tests/fast/spark/test_spark_writer.py:139).
   Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.
   Keep the names PySpark accepts in the shared loop, pin the shim's wider acceptance of `x.y` and `ab`` underif not USE_ACTUAL_SPARK` (or assert PySpark's refusal there), and say in the description that a dot or a backtick in a table name is the shim's leniency.

2. No test tells reading a backtick quoted dotted name as one part apart from passing the joined string, because no schema x exists in the test (tests/fast/spark/test_spark_dataframe.py:940).
   Severity: minor. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.
   Create a schema x before the loop, so `x.y` must be read as one part.

Smaller things

• The comment over _SHOW_PRAGMA (duckdb/_relation.py:76) says a look-alike letter is not a keyword, but no test pins it: with the (?a:...) groups dropped and no ASCII flag at all, every touched test passes. A pragma "ſhow"('t') case beside the Unicode space test would pin what the comment states.
• pytest.raises(Exception, match=...) at tests/fast/spark/test_spark_writer.py:142 accepts any exception; the temporary view test names (AnalysisException, ParseException), which fits here too.
• A behaviour change the description implies with "double quotes are ordinary characters" but does not spell out for existing callers: spark.table('"orders"') read the table orders on 2.0.0 and the base and now raises CatalogException for a table whose name holds the quotes; saveAsTable('"new2"') created new2 and now creates a table named "new2" with the quotes; saveAsTable('"orders"') beside an existing orders raised "already exists" and now creates a second table. PySpark 4.3.0 raises ParseException for all three, so refusing a double quote in a table name would be closer to Spark than either behaviour. That is the maintainer's call; the head applies one rule to temporary views, spark.table and saveAsTable, which the last round accepted for temporary views. Listing it among the behaviour changes is enough for this pull request.

Outside findings not taken

• Codex, "existing double quoted names now refer to different objects": verified (the probe above) and listed as a behaviour change rather than a fix request, because PySpark itself refuses these names and the head reads all three Spark entry points by one rule.

Codex's other finding, the saveAsTable test under PySpark, is finding 1, confirmed by running the test against PySpark 4.3.0 rather than by reading Spark's bytecode.

What I verified

• Head 1d10a6e; range 5b8e7e4..1d10a6e (13 files, 864 added, 168 removed); since the last round a7a998a..1d10a6e (8 files, Python and tests only).
• Builds: the head (Release, the review script's build); the merge base (Release, unchanged since the last round, the merge base did not move); duckdb 2.0.0 from PyPI; a clone of the head rebuilt with the C++ mutation above.
• Probes on 2.0.0, the base and the head: the first round's eleven probes and the last round's six rerun (including the appender against relation writes, con.table with tuples, Enum member parts, empty tuple parts inside BEGIN); new ones for spark.table and saveAsTable over 26 names, pragma "show" with Unicode spaces, case and the long s, the same names on real PySpark 4.3.0 (Temurin 17, local[2]), and the joined string weaker fix.
• Tests on the head, tests/fast/relational_api, tests/fast/api/test_table_name_resolution.py, tests/fast/api/test_seam.py and tests/fast/spark at -n 8: 4061 passed, 40 skipped, 13 xfailed in 63.61s. The new and changed tests on the merge base: 6 failed, 4 passed (the two tuple storage cases and the two Unicode space cases pass, as they should). Against PySpark 4.3.0 with USE_ACTUAL_SPARK=true, the two touched Spark files: 2 failed, 106 passed, 1 skipped; the failures are the new saveAsTable test and test_df_from_pandas, which fails on PySpark's pandas 3 FutureWarning and is not touched here.
• One area reverted at a time on the head, touched tests (1073 pass at the head): SparkSession.table with the raw string, 2 failed; saveAsTable with the raw string, 1 failed; spark.table with the joined parts, 0 failed (finding 2); the last round's pragma pattern, 2 failed; the pattern without ASCII groups, 0 failed (smaller thing); the empty temporary view check removed, 1 failed; a tuple taking the literal fall back, 1 failed; the C++ check by the default database, 4 failed.
• Outside: Codex at xhigh, read only, on the delta: two findings, one taken as finding 1, one listed as a behaviour change. The code-review skill was not run for a delta of this size.
• CI on 1d10a6e at review time: 7 checks passed, 5 pending (two macOS seed jobs, the sdist and two manylinux sanity jobs), none failed.

What held

• Every name createOrReplaceTempView accepts reads back through spark.table, with and without a schema named by its first part; spark.table keeps reading s.t, S.T, memory.s.t, an attached f.ft and f.main.ft, and the CSV file people.csv as 2.0.0 did, and now reads `my table` and `orders` as PySpark does.
• The file database check follows the database the name names, both directions, by string and by tuple, and leaves no view behind when it refuses.
• The pragma "show" rewrite agrees with the engine on every spelling probed: ASCII, no-break and ideographic spaces, upper case, the long s, a trailing semicolon, surrounding spaces.

[el-marvin]

The new saveAsTable test fails against PySpark, which refuses the table names it creates.

Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.

for name in ["`x.y`", "s.t", "`a``b`"]:
    df.write.saveAsTable(name)
    assert [tuple(r) for r in spark.table(name).collect()] == [(1,)]

The loop runs under USE_ACTUAL_SPARK=true too; only the catalog listing below it is guarded. Spark's session catalog checks a permanent table name against ([\w_]+) (SessionCatalog.validateName, called from createTable). With pyspark 4.3.0 and a Java 17 runtime:

USE_ACTUAL_SPARK=true pytest tests/fast/spark/test_spark_writer.py::TestRepeatedColumnNames
E   pyspark.errors.exceptions.captured.AnalysisException: [INVALID_SCHEMA_OR_RELATION_NAME] `x`.`y` is not a valid name for tables/schemas. Valid names only contain alphabet characters, numbers and _. SQLSTATE: 42602
1 failed, 8 passed

PySpark also refuses saveAsTable("`ab")`` (INVALID_ATTRIBUTE_NAME_SYNTAX) and accepts s.tand ``ok_name``. The temporary view test passes against PySpark, since Spark does not check temporary view names this way. The shim creatingmain."x.y"is a leniency it may keep, but the test states it as Spark's behaviour, and so does the description's "a view or table made under ``x.y` `` reads back".

Keep Spark-valid names in the shared loop (s.t, `t1`, `s`.`t2`), move `x.y` and `ab`` underif not USE_ACTUAL_SPARK` (or assert PySpark's refusal there), and adjust that sentence of the description.

[el-marvin]

No test tells reading a backtick quoted dotted name as one part apart from passing the joined string, because no schema x exists here.

Severity: minor. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.

With SparkSession.table changed to self.conn.table(".".join(_name_parts(tableName))), every touched test passes (1073 passed): con.table("x.y") falls back to the literal name x.y while no schema x exists. Once a schema x exists, the joined form reads x.y as schema and table, and the head still reads the view:

== joined variant
no schema x:   ok [Row(a=1)]
with schema x: CatalogException: Catalog Error: Table with name y does not exist!
== head
no schema x:   ok [Row(a=1)]
with schema x: ok [Row(a=1)]

A spark.sql("CREATE SCHEMA IF NOT EXISTS x") before the loop (it runs under PySpark as well) makes the dotted name have to be read as one part.

[el-marvin]

Changes requested.

I reviewed f8f26ab, one commit on top of the last round's head 1d10a6e that changes tests only: three test files, 32 lines added and 5 removed, and nothing under duckdb/ or src/ (git diff 1d10a6e f8f26ab on those two paths is empty). I ran it on the Release build of the head, the Release build of the merge base 5b8e7e4 and the released duckdb 2.0.0, and ran the Spark tests both against the shim and against PySpark 4.3.0 on a Java 17 runtime. The premise still holds: 2.0.0 and the merge base put create_view("s.v") in the default schema under the dotted name, the head puts it in schema s; the approach sits at the right layer, as the earlier rounds found. Every item of the last review is answered and checks out: the saveAsTable test passes against PySpark, the temporary view test now fails for the joined string reading, the long s test fails under the merge base's pattern, and the description's double quote sentence matches what the builds do. One thing must change before merging: under PySpark the two saveAsTable tests leave their tables in the warehouse directory, so the second run in the same checkout fails both. The fix is a teardown in the test file.

Since the last review

• Round 3, the saveAsTable test failed against PySpark: addressed on a first run. With USE_ACTUAL_SPARK=true, test_spark_writer.py and test_spark_dataframe.py give 1 failed, 108 passed, 1 skipped (last round: 2 failed, 106 passed, 1 skipped); the one failure left is test_df_from_pandas on PySpark's pandas 3 FutureWarning, untouched here. The two TestRepeatedColumnNames classes alone: 31 passed. The description now says PySpark refuses a dot or a backtick inside a permanent table name and that the shim keeps taking them. A second run is finding 1 below.
• Round 3, the joined string reading passed the temporary view test: addressed. self.conn.table(".".join(_name_parts(tableName))) in a clone of the head turns test_temp_view_name_is_read_with_spark_quoting red, and the new test_save_as_table_takes_a_dot_or_a_backtick_in_a_quoted_name as well.
• Round 3, no test for the long s rule: addressed. The merge base's pattern (re.IGNORECASE without the ASCII groups) in a clone turns both cases of test_pragma_show_with_a_long_s_is_refused_like_sql red, and both fail on the merge base build, where rel.query describes the relation while SQL raises "Pragma Function with name ſhow does not exist!".
• Round 3, pytest.raises(Exception, ...): addressed, now (AnalysisException, ParseException); PySpark raises ParseException for "t.", the shim AnalysisException.
• Round 3, the double quote change was not spelled out: addressed, and the sentence holds. On 2.0.0 and the merge base spark.table('"orders"') reads orders; on the head it raises CatalogException ("Table with name "orders" does not exist!"). saveAsTable('"new2"') made new2 before and makes "new2", quotes included, now. PySpark 4.3.0 raises ParseException for both.
• Rounds 1 and 2: every earlier probe rerun on 2.0.0, the merge base and the head prints what it printed last round (one differs only in object addresses), and the earlier reverts still turn their tests red: spark.table with the raw string (3 failed), saveAsTable with the raw string (2), the round 2 re.ASCII pattern (2), no empty name check (1), the tuple taking the literal fall back (1).

Findings

1. Under PySpark the two saveAsTable tests leave their tables on disk, so every later run in the same checkout fails them (tests/fast/spark/test_spark_writer.py:139).
   Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.
   Make both tests leave nothing in the session or the warehouse under USE_ACTUAL_SPARK=true, and check it with two runs of the class in a row. Evidence in the inline comment.

Smaller things

• The comment at tests/fast/spark/test_spark_writer.py:149 names INVALID_SCHEMA_OR_RELATION_NAME for both names, and the description gives SessionCatalog.validateName as the reason PySpark refuses "a dot or a backtick". PySpark 4.3.0 refuses `ab`` withINVALID_ATTRIBUTE_NAME_SYNTAX ("Syntax error in the attribute name: ab"), from its name parser, and the PySpark branch of the test asserts only `x.y`. Both are AnalysisException, so the refusal holds; the named cause fits the dot only.

Outside findings not taken

• Codex, "the leftover t1 breaks TestSparkCatalog.test_list_tables": confirmed by running, and folded into finding 1; the order dependence itself is older, since temporary views from tests the merge base already had break that test the same way.

Codex's finding on the leftover tables is finding 1, confirmed by two consecutive runs against PySpark; its finding on the error class in the comment is the smaller thing above.

What I verified

• Head f8f26ab, merge base 5b8e7e4, delta 1d10a6e..f8f26ab: tests/fast/relational_api/test_rapi_query_binding.py, tests/fast/spark/test_spark_dataframe.py, tests/fast/spark/test_spark_writer.py.
• Against the shim on the head at -n 8: the three changed files, 175 passed; tests/fast/relational_api, tests/fast/api/test_table_name_resolution.py, tests/fast/api/test_seam.py and tests/fast/spark, 4064 passed, 40 skipped, 13 xfailed (last round 4061, plus the three new cases).
• Against PySpark 4.3.0 (Temurin 17, master local[4], fresh warehouse): the two Spark files 1 failed, 108 passed, 1 skipped; the two changed classes 31 passed. Leftovers: the writer class then test_list_tables in one run, 1 failed, 8 passed; the writer class again without deleting the warehouse, 2 failed, 6 passed; the merge base's tests of the two files then test_list_tables, 2 failed (test_df_from_pandas and test_list_tables on temporary views), 105 passed, warehouse left empty.
• The five new and changed tests on the merge base build: 5 failed.
• Reverts in a clone of the head, over the 1076 tests last round's set covers: the head 1076 passed, 3 skipped; each revert listed above turns its tests red.
• Probes on 2.0.0, the merge base and the head: every probe of rounds 1 to 3 rerun; a new one for the long s, which gives CatalogException "Pragma Function with name ſhow does not exist!" from SQL and from rel.query on the head for both spellings.
• Codex at extra high effort, read only, on the delta. The code-review skill was not run for a tests only delta of this size.
• CI on f8f26ab at 23:33 UTC: 8 passed, 4 pending (macOS arm64 seed, the two manylinux sanity jobs, the sdist), none failed.

What held

• The long s test cannot pass for an unrelated reason: table t exists, both paths raise the same class with the same first line, and under the merge base's pattern rel.query succeeds, so the test fails at its pytest.raises.
• With schema x present, the joined reading turns `x.y` into schema x, table y and misses the view, so the temporary view test now tells the two readings apart.
• Against the shim each test gets its own connection, so the two writer tests do not see each other's tables, and both pin the full table listing.
• PySpark accepts s.t, `t1` and `s`.`t2` and reads each back through spark.table, refuses "t." with ParseException and `x.y` with AnalysisException, as the tests now say.

[el-marvin]

Under PySpark the two saveAsTable tests leave their tables on disk, so every later run in the same checkout fails them.

Severity: should fix. Category: correctness. Evidence: ran. Fix: root-cause. Origin: duckdb-python. Base: introduced.

PySpark's default session catalog keeps a managed table's files under spark-warehouse in the working directory, and the spark fixture in tests/conftest.py returns one shared session for the whole run. This test saves s.t, `t1` and `s`.`t2` and never drops them:

        for name in ["s.t", "`t1`", "`s`.`t2`"]:
            df.write.saveAsTable(name)

and the next test's refused saveAsTable("`x.y`") (line 154) still writes its files before the catalog refuses the name. After one run of TestRepeatedColumnNames in this file with USE_ACTUAL_SPARK=true (PySpark 4.3.0, Java 17), the warehouse holds s.db/t, s.db/t2, t1 and x.y, each with parquet files. A second run in the same checkout:

SparkRuntimeException: [LOCATION_ALREADY_EXISTS] Cannot name the managed table as `spark_catalog`.`s`.`t`, as its associated location '.../spark-warehouse/s.db/t' already exists. Please pick a different table name, or remove the existing location first. SQLSTATE: 42710
SparkRuntimeException: [LOCATION_ALREADY_EXISTS] Cannot name the managed table as `spark_catalog`.`default`.`x.y`, as its associated location '.../spark-warehouse/x.y' already exists. ...
2 failed, 6 passed

The second one is not an AnalysisException, so the pytest.raises(AnalysisException) at line 153 does not catch it. The Spark tests the merge base already had leave the warehouse empty (this file and test_spark_dataframe.py under PySpark with the three name tests deselected: nothing under spark-warehouse afterwards), and spark-warehouse is not in .gitignore, so the leftovers also show in git status. Within one run the leftover t1 is listed by spark.catalog.listTables(): run after this class, TestSparkCatalog.test_list_tables fails on Table(name='t1', ..., tableType='MANAGED', isTemporary=False). That test is already order dependent through temporary views other tests leave (repeated_filter_view), so that part is not new.

The repository's idiom is the table_name fixture in tests/fast/spark/test_spark_session.py, which drops its table in teardown. Drop s.t, t1 and s.t2 when the test ends, and for the refused `x.y` remove the location Spark wrote under spark.conf.get("spark.sql.warehouse.dir"), or assert the refusal in a way that writes nothing. Fixed when two consecutive runs of the class with USE_ACTUAL_SPARK=true pass without deleting spark-warehouse, and no table directory is left in it.

[el-marvin]

This is ready to merge. I reviewed 67cce9b, one commit on top of round 4's f8f26ab that changes one test file, tests/fast/spark/test_spark_writer.py (30 lines in, 4 out); nothing under duckdb/ or src/ changed, so the package is the one round 4 reviewed. The premise still holds on the new build: 2.0.0 and the merge base put create_view("s.v") at memory.main."s.v", the head puts it at memory.s.v. The approach sits at the right layer, the engine's own QualifiedName::TryParse behind one seam entry, as settled in round 2. Round 4's one should fix is addressed and I confirmed it under real PySpark 4.3.0, not only in the shim. Nothing here blocks merging. The items below are nits the author may take or leave. Outside the code the maintainer still has to let the five pending CI jobs finish (none has failed) and accept the behaviour changes the description lists, which earlier rounds raised and did not block.

Since the last review

• Addressed: round 4's should fix, the two saveAsTable tests leaving s.t, t1, s.t2 and the refused x.y under spark-warehouse so that a second run in the same checkout failed with LOCATION_ALREADY_EXISTS. With USE_ACTUAL_SPARK=true, a fresh warehouse, and nothing deleted between the runs, TestRepeatedColumnNames passed 8 of 8 twice in a row, and after each run only the empty spark-warehouse directory remained, as the merge base's tests leave it. The writer class followed by test_spark_catalog.py::TestSparkCatalog::test_list_tables passed 9 of 9, where round 4 saw t1 leak into the listing. With an assert False forced into both tests after their saves and refusals, the teardown still ran and the warehouse was empty after each of two runs. Removing only the shutil.rmtree line brought back the failure (LOCATION_ALREADY_EXISTS ... default.x.y on the second run) and left x.y and a`b behind. So the location removal is what makes the second run clean, as the reply says, and PySpark writes the files for both refused names before refusing them, as the fixture's comment says. In shim mode the class passed twice with no spark-warehouse created.
• Addressed in part, with a correction to round 4: the cause of the `ab `` refusal. The comment and the description now name each refusal with its error class, and the test asserts both classes (INVALID_SCHEMA_OR_RELATION_NAMEfor ``x.y``,INVALID_ATTRIBUTE_NAME_SYNTAXfor ``ab` ), which PySpark 4.3.0 raises. My round 4 review said `ab`` was refused for a different cause than the one named. That was wrong: the Java stack trace shows both names are refused by the same check,SessionCatalog.validateName(called fromSessionCatalog.createTable, after the data is written). The error class differs only because, while building the INVALID_SCHEMA_OR_RELATION_NAMEmessage,QueryCompilationErrors.invalidNameForTableOrDatabaseErrorcallstoSQLId, which parses the name again with AttributeNameParser` and fails on the lone backtick. The part that remains, "from its name parser", is the first item below.
• Rounds 1 to 3: every earlier probe gives the same output on the new build as in round 4, apart from object addresses and script paths. The revert variants give the same results: spark_table_joined, spark_table_raw, save_as_table_raw and pragma_no_ascii each turn the tests that pin them red, and the head passes 1076 of the touched tests.

Findings

None.

Smaller things

• The comment at tests/fast/spark/test_spark_writer.py:171 and the description's Spark paragraph attribute the backtick refusal to Spark's "name parser". The name parser saveAsTable uses accepts `ab `` (PySpark writes `` ab `` to disk), and the refusal is SessionCatalog.validateName, the same check as for the dot. Something like "Spark's `SessionCatalog.validateName` refuses a dot or a backtick in a table name after writing the data; for the backtick the error class is INVALID_ATTRIBUTE_NAME_SYNTAX, because building the message parses the name again" would say what happens. The error classes asserted are right either way.
• tests/fast/spark/test_spark_dataframe.py:941 (added in round 4) creates schema x and does not drop it. Under actual Spark this leaves schema x in the shared session and an empty x.db under spark-warehouse. It does no harm: a second run passes, and the writer fixture drops x when it runs later. A DROP SCHEMA IF EXISTS x at the end, or the same fixture shape, would leave the warehouse as the merge base's tests leave it.

Outside findings not taken

• Codex (xhigh, read only, on the delta) reported no findings above the bar. It noted that other Spark tests still leave temporary views in the shared session. That is true and pre-existing (round 4 measured it at the merge base), and it is outside what this change claims.

What I verified

• Head 67cce9b; merge base 5b8e7e4 (unchanged); delta git diff f8f26ab..67cce9b touches one file, tests/fast/spark/test_spark_writer.py, 30 insertions, 4 deletions. The pull request diff on GitHub carries the same hunks.
• Builds: the head as a Release build by the review script (log ends BUILD OK, tree HEAD 67cce9b); the merge base Release build from round 2; the 2.0.0 release in its own environment.
• Shim mode: TestRepeatedColumnNames twice in a row, 8 passed each time, no spark-warehouse created. Touched directories (tests/fast/relational_api, tests/fast/api/test_table_name_resolution.py, tests/fast/api/test_seam.py, tests/fast/spark) at 8 workers: 4064 passed, 40 skipped, 13 xfailed.
• Real PySpark 4.3.0 on a Temurin 17 JDK (USE_ACTUAL_SPARK=true, local master with 4 threads, pytest without xdist), from a fresh warehouse that was not cleaned between runs: the writer class twice, 8 passed and 8 passed, warehouse empty after each; the writer class then TestSparkCatalog::test_list_tables, 9 passed; test_spark_writer.py and test_spark_dataframe.py, 1 failed (test_df_from_pandas, PySpark's pandas 3 FutureWarning, untouched and the same in round 4), 108 passed, 1 skipped; the dataframe quoting test followed by the writer class twice over the warehouse that run left (with x.db), 9 passed each time.
• Teardown on failure: a copy of the file with assert False after the saves in the first test and after the dotted refusal in the second, run twice under real PySpark: 2 failed with the forced assertions both times, warehouse empty after each. A copy without the shutil.rmtree call: the first run passed and left x.y and a`b; the second failed with LOCATION_ALREADY_EXISTS for default.x.y.
• The backtick refusal: a script saving `ab`` under PySpark printed the Java framesAttributeNameParser.parseAttributeName<DataTypeErrorsBase.toSQLId<QueryCompilationErrors.invalidNameForTableOrDatabaseError<SessionCatalog.validateName(SessionCatalog.scala:325) <SessionCatalog.createTable(SessionCatalog.scala:577) <CreateDataSourceTableAsSelectCommand.run, and `` ab `` was on disk afterwards.
• Earlier rounds: the probes of rounds 1 to 4 rerun on the release, the base and the head; the revert variants rerun in a clone at the head (results above).
• Outside review: Codex at xhigh effort, read only, on the delta; no question filed.
• CI on 67cce9b: 7 passed (submodule sanity, code quality, the Windows amd64 and arm64, manylinux x86_64 and aarch64, and macOS arm64 seeds), 5 pending (macOS x86_64 seed, sdist, two manylinux sanity jobs, the full API build with assertions), none failed.

What held

• The fixture asks for spark, so its teardown runs while the session is alive, and it runs after a failing test body too. Tables go before their schemas, so DROP SCHEMA IF EXISTS without CASCADE succeeded every time, including after the forced failures.
• DROP TABLE IF EXISTS with the backtick quoted names is accepted by both PySpark and the shim, whether the table exists or not.
• The warehouse location is read from spark.sql.warehouse.dir (a file: URL here) and resolves to the directory PySpark writes to. Only the two named leftovers are removed.
• Order: the dataframe test before the writer tests, the writer tests before the catalog listing, and repeated runs over a dirty warehouse all passed.