Invalid JOIN/ON reconstruction

[coreybutler]

I'm using this library to parse DDL scripts and validate/document individual statements before running them. This library does a great job of splitting the statements into a slice of Statements. However; since there is no way to access the original statement, we rely on the reconstructed statement (i.e. Statement.String()). I've encountered an issue where the reconstructed JOIN/ON clauses are inaccurate.

The core of the problem (original code vs reconstructed code):

Notice the command sequence in the last three lines: JOIN JOIN ON ON

- FROM worker w
- JOIN event_worker ew ON ew.worker_id = w."id"
- JOIN sys_event se ON se."id" = ew."event"
+ FROM "worker" AS "w"
+ JOIN "event_worker" AS "ew"
+ JOIN "sys_event" AS "se" ON "se"."id" = "ew"."event"
+ ON "ew"."worker_id" = "w"."id"

Here is a snippet of the Go code I'm using (this module aliased as rsql) that stores statements in a slice for later use:

var stmt rsql.Statement
parser := rsql.NewParser(strings.NewReader(s))
statements := make([]rsql.Statement, 0)
ct := 0

for {
  stmt, err = parser.ParseStatement()
  if err != nil {
    if fmt.Sprintf("%s", err) == "EOF" {
      break
    }

    os.WriteFile("debug.sql", []byte(s), os.ModePerm)
    panic(fmt.Errorf("SQL parsing error:%s", err.Error()))
  } else {
    statements = append(statements, stmt)
    ct++
  }
}

Here is one of the source statements that is not being reconstructed properly:

CREATE TRIGGER IF NOT EXISTS tr_worker AFTER INSERT
 ON audit
 FOR EACH ROW
 WHEN "nm" NOT LIKE 'WORKER_%' AND "nm" != 'WEBHOOK_EVENT'
BEGIN
  INSERT INTO audit ("id", "nm", "data", "source_id", "source_table")
  SELECT
      lower(hex(randomblob(16)))
    , 'WORKER_EVENT'
    , json_object(  
        'worker', w."id",
        'data', NEW."data"
      )
    , NEW."id"
    , 'audit'
  FROM worker w
    JOIN event_worker ew ON ew.worker_id = w."id"           --- NOTICE HERE ---
    JOIN sys_event se ON se."id" = ew."event"               --- AND HERE    ---
  WHERE se."id" = NEW."nm"
    AND coalesce(se."expire_dt", datetime(CURRENT_TIMESTAMP, '+1 month')) >= CURRENT_TIMESTAMP
    AND coalesce(se."purge_dt", datetime(CURRENT_TIMESTAMP, '+1 month')) >= CURRENT_TIMESTAMP
    AND CURRENT_TIMESTAMP >= w."valid_start_dt"
    AND CURRENT_TIMESTAMP <= coalesce(w."valid_end_dt", datetime(CURRENT_TIMESTAMP, '+1 month'))
    AND (
      w."dsn_id" IS NULL
      OR coalesce(json_extract(NEW."data", '$.dsn'), '__NONE__') = w."dsn_id" 
    )
  ORDER BY ew."exec_order" ASC; 
END
;

This is how it's being reconstructed (i.e. output of stmt.String(), formatted for readability):

CREATE TRIGGER IF NOT EXISTS "tr_worker"
AFTER INSERT ON "audit"
FOR EACH ROW
WHEN "nm" NOT LIKE 'WORKER_%' AND "nm" != 'WEBHOOK_EVENT'
BEGIN
  INSERT INTO "audit" ("id", "nm", "data", "source_id", "source_table")
  SELECT
    lower(hex(randomblob(16))),
    'WORKER_EVENT',
    json_object(
      'worker', "w"."id",
      'data', "NEW"."data"
    ),
    "NEW"."id",
    'audit'
  FROM "worker" AS "w"
  JOIN "event_worker" AS "ew"                                   --- MAIN ISSUE IS 
  JOIN "sys_event" AS "se" ON "se"."id" = "ew"."event"          --- <-- HERE
  ON "ew"."worker_id" = "w"."id"                                --- <-- AND HERE
  WHERE
    "se"."id" = "NEW"."nm"
    AND coalesce("se"."expire_dt", datetime("CURRENT_TIMESTAMP", '+1 month')) >= "CURRENT_TIMESTAMP"
    AND coalesce("se"."purge_dt", datetime("CURRENT_TIMESTAMP", '+1 month')) >= "CURRENT_TIMESTAMP"
    AND "CURRENT_TIMESTAMP" >= "w"."valid_start_dt"
    AND "CURRENT_TIMESTAMP" <= coalesce("w"."valid_end_dt", datetime("CURRENT_TIMESTAMP", '+1 month'))
    AND (
      "w"."dsn_id" IS NULL
      OR coalesce(json_extract("NEW"."data", '$.dsn'), '__NONE__') = "w"."dsn_id"
    )
  ORDER BY "ew"."exec_order" ASC;
END

[otoolep]

Thanks for the report. Could you supply a much simpler SQL statement showing the issue? That way we can turn it into a unit test.

[coreybutler]

Here's a much simpler recreation:

package main

import (
  "fmt"
  "strings"

  "github.com/rqlite/sql"
)

func main() {
  query := `
    SELECT *
    FROM table_a as a
      JOIN table_b as b ON a.id = b.id
      JOIN table_c as c ON b.id = c.id
    ;
  `

  parser := sql.NewParser(strings.NewReader(query))

  for {
    stmt, err := parser.ParseStatement()
    if err != nil {
      if err.Error() == "EOF" {
	break
      }
      panic(err)
    }

    fmt.Printf("Reconstructed statement as: \n%s\n", stmt.String())
  }
}

Results in my terminal...

---

Actual vs. Expected

For copy/paste purposes:

Actual Output

SELECT * FROM "table_a" AS "a" JOIN "table_b" AS "b" JOIN "table_c" AS "c" ON "b"."id" = "c"."id" ON "a"."id" = "b"."id"

Expected Output

SELECT * FROM "table_a" AS "a" JOIN "table_b" AS "b" ON "a"."id" = "b"."id" JOIN "table_c" AS "c" ON "b"."id" = "c"."id" 

[otoolep]

Thanks @coreybutler - I don't suppose you've looked into the code to see where it might be going wrong? :-) Could be something simple.

[coreybutler]

@otoolep I glanced, but I didn't have a chance to really dig in or run any tests. I suspect the starting point for debugging is

sql/ast.go

Lines 3301 to 3304 in a4122fa

	fmt.Fprintf(&buf, "%s%s%s", c.X.String(), c.Operator.String(), c.Y.String())
	if c.Constraint != nil {
	fmt.Fprintf(&buf, " %s", c.Constraint.String())
	}

My gut, which could be very wrong, tells me it could be how the join constraint is being populated? Perhaps the JOIN and ON clauses are somehow being concatenated, tricking the String() into thinking it is writing the output correctly. Perhaps something along the lines of fmt.FPrintf(&buf, "%s %s", joinStr, onStr) where joinStr is JOIN table_a as a JOIN table_b as b and onStr is ON a.id = b.id ON c.id = b.id? I'm nowhere near familiar enough with the code to actually know what I'm talking about, but that seems like something that would be really easy to unwittingly do.