Skip to content

[#589] DELIM "" at the column level invalidates any PIECE specified and fetches entire node

Narayanan Iyer requested to merge nars1/YDBOcto:octo589 into master

Functional Spec

When the DELIM keyword is supplied the value "" (i.e. the empty string) and used at a column level in a CREATE TABLE specification, fetching that column in the SELECT column list of a query used to return the empty string as it did a $PIECE() of a node value using "" as the piece delimiter (it does not matter what the piece number is in this case).

This is not in any way helpful to the user so it is now changed to return the entire node value (i.e. not do any $PIECE() in the generated plan).

If a PIECE keyword was also specified for the same column in the CREATE TABLE command, that is now ignored as it makes no sense.

Since no user is expected to have been relying on the prior DELIM "" behavior (to return an empty string), it is not expected to affect any user even though it might seem a backward incompatible change.

Code changes

  • src/assign_table_to_columns.c : Checks if a DELIM of "" is specified for a column and a PIECE has been specified. If so, it removes PIECE from the list of specified keywords (i.e. discards it).

  • src/emit_column_specification.c : Because of the changes in the previous bullet, we now have asserts here that we never see an empty DELIM and a PIECE specified for the same column.

  • src/m_templates/tmpl_column_reference.ctemplate : This is now modified to handle the case where PIECE keyword might be missing for a non-key column too. In that case, it generates M code which does not do a $PIECE($GET(...)) but instead just does the ... part.

  • src/octo.h : New DEBUG_ONLY and NDEBUG_ONLY macros are now introduced to be used for code that is specific to Debug and Release builds respectively. This avoids using a #ifdef NDEBUG or #ifndef NDEBUG block in code which makes it less readable due to the need to keep the # at the first column in the line. Various NDEBUG usages have now been replaced with the new macros.

  • A new test_createtable/TC037 bats subtest verifies this change.

  • Note that there is no need to bump either FMT_BINARY_DEFINITION or FMT_PLAN_DEFINITION in src/octo.h because even if the text table definition was created by an older build of Octo which did not do the removal of PIECE, when a find_table() call is done by the newer build of Octo, it would be invoking assign_table_to_columns() which will take care of removing PIECE from the keyword list for applicable columns. This will also change the hash signature of the table and therefore result in different physical plan names for the same query compared to older Octo builds i.e. no issues to worry about in terms of auto upgrade since different plans are generated automatically in case pre-existing tables did have a DELIM "" specification along with a PIECE keyword (very unlikely in practice).

Merge request reports