go-jet / jet

Type safe SQL builder with code generation and automatic query result data mapping
Apache License 2.0
2.59k stars 118 forks source link

Generating schema Fails #408

Open safaci2000 opened 1 week ago

safaci2000 commented 1 week ago

Describe the bug The code generator should fail if datamodel contains reserved fields.

There is a collision with the way the code is constructed, because postgres.Table exposes TalbeName() and the table contains a field named TableName the code does not compile.

Environment (please complete the following information):

Code snippet

create schema audit;
drop table if exists audit.audit_actions;
create table audit.audit_actions
(
    schema_name text not null,
    table_name text not null,
    trx_id text default txid_current(),
    user_name text,
    app_user text,
    action TEXT NOT NULL,
    old_data jsonb,
    new_data jsonb,
    query text,
    creation_date timestamp with time zone DEFAULT now() NOT NULL
) with (fillfactor=100);

Expected behavior Generated code to compile successfully or fail hard on the generation and provide context on the reason for the failure

go-jet commented 1 week ago

I guess we could add _ after each table column named TableName, SchemaName or Alias.
In the meantime, workaround would be to customize generator.

safaci2000 commented 1 week ago

I think the point I was getting at is that I think it should fail and detect a failure rather than generate code that doesn't compile. You could potentially run into the same issue with table_ or such. Maintain a reserve words that are part of your model if they're not overriden via the custom config.

go-jet commented 1 week ago

I think the point I was getting at is that I think it should fail and detect a failure rather than generate code that doesn't compile.

The only way the generator can detect build failure is to initiate go build. I think that would be too much responsibility for a generator. Also, it would probably trigger every anti-virus software out there.

You could potentially run into the same issue with table_ or such.

True, but there is always an option to customize the generator.

Maintain a reserve words that are part of your model if they're not overriden via the custom config.

Since the default generator always generates columns in PascalCase, the only possible conflict names are Table, TableName, SchemaName, Alias, AllColumns, and MutableColumns. If we handle these cases, the only way for users to get more conflicts is by customizing the generator.

safaci2000 commented 1 week ago

Okay, maybe we should add a note in the wiki to call that out? So, detecting a build failure is likely overkill, what I was getting at is ensuring those values are not used.

Since we have a finite set of reserved words, can't the generator fail if it finds a column name titled one of those values you listed above?

It does add a check to check on every field but it's a better user experience than trying out jet after never hearing about it and having it blow up and moving on since it's obviously 'broken'.

go-jet commented 1 week ago

Since we have a finite set of reserved words, can't the generator fail if it finds a column name titled one of those values you listed above?

It doesn't have to fail. If generator encounters any of above listed values, generator can add any character and thus break the name conflict. For instance if we add _:

type auditActions struct {
    postgres.Table

    // Columns
    TableName_    postgres.ColumnString
}
safaci2000 commented 5 days ago

Ah, so dynamic renaming naming... yeah that'd be much better. 👍