stephenafamo / bob

SQL query builder and ORM/Factory generator for Go with support for PostgreSQL, MySQL and SQLite
https://bob.stephenafamo.com
MIT License
701 stars 37 forks source link

Handle `null` column names in expression indexes #246

Closed mbezhanov closed 5 days ago

mbezhanov commented 5 days ago

This provides a fix to #244.

DDL for testing:

CREATE TABLE t1 (
  col1 int,
  col2 int,
  col3 int
);

CREATE INDEX idx1 ON t1 ((col1 + col2));
CREATE INDEX idx2 ON t1 ((col1 + col2), col3);
CREATE INDEX idx3 ON t1 (col3);

Before the patch, generation failed with the following error:

  1. MySQL:

    2024/06/27 11:49:12 unable to fetch table data: unable to load indexes: sql: Scan error on column index 2, name "column_name": converting NULL to string is unsupported

  2. SQLite:

    2024/06/27 11:51:14 unable to fetch table data: sql: Scan error on column index 0, name "name": converting NULL to string is unsupported

After the patch, generation completes successfully with the following information being collected:

image


Please note that PostgreSQL was not affected by this issue, as it uses the expression as a column_name rather than omitting it:

image


@stephenafamo would you prefer to use the expression contents as a column_name for SQLite and MySQL as well, rather than omitting them? I checked two different database GUIs. The first one used the expression contents as a column_name, and the others one omitted the expression from the column listing.

stephenafamo commented 5 days ago

I think it would be better to have a different field in the struct to contain the expression.

This way, someone could generate code to use the expression in a query.

mbezhanov commented 5 days ago

I've updated the PR accordingly.

How do you feel about this format for the Index structures:

image

That corresponds to the following DDL:

CREATE TABLE test_index_expressions (
    col1 int,
    col2 int,
    col3 int
);
CREATE INDEX idx11 ON test_index_expressions (col1, (col2 + col3), (POW(col3, 2)))
stephenafamo commented 5 days ago

This looks good 🎉

Perhaps it makes sense to have a flat struct for Index

type Index struct {
  Name string
  Columns []string
  Expressions []string
}
mbezhanov commented 5 days ago

Perhaps it makes sense to have a flat struct for Index

:white_check_mark: Done