Skip to content

fix(python): render field types as SQL strings, not Rust Debug - #786

Open
jackylee-ch wants to merge 3 commits into
apache:mainfrom
jackylee-ch:fix/data-type-sql-string
Open

fix(python): render field types as SQL strings, not Rust Debug#786
jackylee-ch wants to merge 3 commits into
apache:mainfrom
jackylee-ch:fix/data-type-sql-string

Conversation

@jackylee-ch

@jackylee-ch jackylee-ch commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

PyDataField.field_type() returns Rust Debug output — a column declared
VARCHAR(10) reads back as Varchar(VarCharType { length: 10, nullable: true }).
The in-place TODO(#284 follow-up) names the cause: DataType has no enum-level
Display, so there was nothing to call.

Only 10 of the 23 variants carried a leaf Display. This adds the other nine
scalars and the four constructed types, then the dispatch. The constructed formats
fill Java's FORMAT with the children's own SQL strings (ARRAY<%s>,
MAP<%s, %s>, MULTISET<%s>, ROW<%s>), so a child's NOT NULL sits inside the
brackets and the outer one after them.

ROW renders fields like DataField.asSQLString: the escaped name, the type, then
COMMENT '...'. So a nested row reads ROW<`id` INT, `name` VARCHAR(10)>.

spec::escape_identifier was not Java's EncodingUtils.escapeIdentifier — it only
doubled " and added no delimiter — so it is aligned here rather than worked
around. It had no production caller before this PR (Display for DataField was
never written), and its test pinned the wrong shape.

The dispatch also retires VectorType::element_sql_name, a private table of seven
primitive names with unreachable!() for the rest.

cargo test -p pypaimon_rust cannot link libpython here, so the binding is
compile-checked locally and its behaviour left to integration (python). The
rendering is covered by unit tests in paimon.

write!(
f,
"{} {}",
crate::spec::escape_identifier(field.name()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Quote ROW field names with Java's identifier syntax

This helper does not implement Java EncodingUtils.escapeIdentifier: the Rust function only doubles " characters and does not add delimiters, while Java returns a backtick-wrapped identifier and doubles embedded backticks. Consequently this renders even id as ROW<id INT> instead of Java's backtick-wrapped id; names containing spaces or reserved words become ambiguous, and a name containing a backtick is not escaped at all. That also means the new Python field_type() contract is not the claimed Java asSQLString() for nested rows.

Please use a ROW-specific helper that wraps the name in backticks and replaces each embedded backtick with two backticks (or deliberately align the existing helper if all its callers need Java syntax), and update the tests to cover a normal name, a reserved/space-containing name, and an embedded backtick.

@jackylee-ch

Copy link
Copy Markdown
Contributor Author

Right — spec::escape_identifier only doubled ". Aligned it with Java's backtick syntax instead of adding a ROW-local escape: it had no production caller and its test pinned the wrong shape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants