Skip to content

[#279] Octo returns incorrect results if key column is the empty string

Narayanan Iyer requested to merge nars1/YDBOcto:octo279 into master
  • src/m_templates/tmpl_tablejoin.ctemplate : The main fix is in this file. And that is to check if before starting the FOR loop for a key column if a STARTINCLUDE keyword was specified. If so we include the START value (that is explicitly specified in the CREATE TABLE or implicitly assumed) for the first check of the FOR loop. And only then do the $ORDER() to move on to further iterations of the loop (like usual). This is done by adding a DO:$DATA(...) before the FOR ... DO in the same M line. This way the new DO also executes the dotted block underneath the same was the pre-existing DO after the FOR executes it.

  • src/m_templates/tmpl_key_source.ctemplate : This was also modified as tmpl_key_source() is now called for one more case from tmpl_tablejoin(). And so it exercised a code path that was buggy but was fine because no one was previously exercising it. This is the case where a cross reference key should not be counted towards the keys_to_match counter. Only primary key columns should be. Hence an additional && !plan_key->is_cross_reference_key check is added before the keys_to_match++.

  • While at this, noticed a few related issues and unreachable code in the following files so removed it.

    • src/m_templates/tmpl_key_advance.ctemplate
    • src/m_templates/tmpl_key_source.ctemplate
  • A new test_primary_key/TPK01 bats subtest verifies this fix. Due to this, the pre-existing vista-mini schema was enhanced to now have actual data sets at least for the ORDER_STATUS table. The data set is miniscule (only a dozen rows or so) but is better than no data as it gives us a test of a VistA environment (where START/END keywords are used in CREATE TABLE) with simulated data.

  • doc/grammar.rst : Pre-existing keywords ADVANCE and CURSOR were removed from the documentation.

    • The ADVANCE keyword was not tested at all. The functionality was not clearly documented either. Not sure if anything was using it. If any the VistA DDL generator should use it but I verified that was not using it either. So removed it as this was functionality that is likely broken and unused so no point continuing to maintain it. That said, I did make changes in the code so we honor this but ignore it otherwise (for backward compatability purposes).
    • The CURSOR keyword was not used anywhere I could find except in the documentation so removed it.

Merge request reports