refactor(output-mapping): create non-typed tables through the table-definition endpoint - #551
Merged
Merged
Conversation
zajca
marked this pull request as ready for review
July 28, 2026 08:51
keboola-pr-reviewer-bot
left a comment
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 3/5) · profile ajda
Needs human: removes a public method and changes observable table-creation behavior in load-bearing platform-libraries, with no feature-flag gate.
Concerns:
libs/output-mapping/src/Storage/TableCreator.php: Public method createTable() removed — potential BC break for library consumerslibs/output-mapping/src/LoadTableTaskCreator.php: distributionKey no longer set on created tables; behavior change with no feature flaglibs/output-mapping/tests/Writer/StorageApiLocalTableWriterTest.php: User-facing invalid-column-name error message changed
odinuv
approved these changes
Jul 29, 2026
odinuv
left a comment
odinuv
left a comment
Member
There was a problem hiding this comment.
může bejt
podle mě ty komentáře, který popisují změnu "// takže vznikne netypovaná tabulka stejně jako dřív, ale bez uploadu hlavičkového CSV souboru" všeobecně nepatří do kódu, ale do PR
| $this->fail('Must throw exception'); | ||
| } catch (InvalidOutputException $e) { | ||
| // the table is created through the table-definition endpoint, which reports an invalid column | ||
| // name without spelling out the length limit |
…efinition endpoint A table declared by manifest `columns` without data types was created with createTableAsync, which needs a header CSV uploaded to file storage first. The same table can be created through the table-definition endpoint by sending the columns without any type - Storage reports a table as typed only when at least one column carries `definition.type` or `basetype` - so the create now goes through createTableDefinition and the file upload disappears. This also aligns the untyped path with the typed and schema paths, which already create tables through the same endpoint. Two observable differences: - distributionKey is no longer set on the created table. Storage documents it as a Synapse-only field, Synapse is gone, and the typed paths do not set it either, so this makes output mapping consistent rather than losing behaviour that works. - an invalid column name is now reported by Storage as `Invalid column name: "..."` instead of naming the 64 character limit.
…on-typed create The non-typed branch now reads primaryKey/columns twice and the definition endpoint reports an invalid column name instead of the length limit.
…anch in the comment
zajca
force-pushed
the
zajca/om-untyped-create-via-table-definition
branch
from
July 29, 2026 08:34
311291d to
707acc6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preparation for AJDA-2946 (#550), useful on its own.
Why
Output mapping has two ways of creating a table. Typed tables and tables declared by
schemago throughcreateTableDefinition. A table declared by manifestcolumnswithout data types went throughcreateTableAsync, which requires a header CSV to be written to a temp file and uploaded to file storage before the table can be created.The definition endpoint can create that table too: Storage reports a table as typed only when at least one column carries
definition.typeorbasetype(BaseCreateTableDefinitionRequest::isTypedTable()), so a payload with plain column names produces a non-typed table. Verified against real Storage — sameisTyped=false, same columns, same primary key, data loads normally.So this switches the untyped path to
createTableDefinition, which:Changes
TableDefinitionFromColumns— builds{name, primaryKeysNames, columns: [{name}]}and nothing else, so no column can accidentally acquire a typeLoadTableTaskCreatoruses it for the!didTableExistBefore() && hasColumns()branchTableCreator::createTable()removed — this change made it unused, and with it the header-CSV plumbing (Temp,CsvFile)Observable differences
distributionKeyis no longer set on the created table. Measured side by side, this is the only field that differs; everything else (isTyped, primaryKey, columns, column definitions, metadata rows, import) is identical.That is acceptable because Storage documents the field as "Distribution key columns (for Synapse)", Synapse is no longer a Storage backend, and on Snowflake it is inert metadata echoed back in the table detail. Note the typed and schema paths already do not set it — so today a typed table created from a config with
distribution_keygets no distribution key while an untyped one does. This makes the two consistent.An invalid column name is reported differently.
createTableAsyncsaid'<name>' is more than 64 characters long; the definition endpoint saysInvalid column name: "<name>". The new message does not spell out the limit.testLongColumnNamewas updated accordingly.Testing
TableCreatorTest— the two tests covering the oldcreateTable()were converted to the new path rather than deleted, so non-typed creation with a primary key and its error handling stay covered against real Storage.TableDefinitionFromColumns, including that a sparse column list (RestrictedColumnsHelper filters witharray_filterand leaves gaps) still serialises as a JSON array.main-writer-tests-1run against real Storage: the only failure caused by this change wastestLongColumnName(message text, updated). The fourCan not resolve branchIddev-branch errors are pre-existing — verified by re-running one of them with these changes stashed, where it fails identically.