Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generating schema Fails #408

Closed
safaci2000 opened this issue Oct 8, 2024 · 6 comments · Fixed by #418
Closed

Generating schema Fails #408

safaci2000 opened this issue Oct 8, 2024 · 6 comments · Fixed by #418
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@safaci2000
Copy link
Contributor

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):

  • OS: mac os X
  • Database: postgres
  • Database driver: pgx
  • Jet version: master

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

@safaci2000 safaci2000 added the bug Something isn't working label Oct 8, 2024
@go-jet
Copy link
Owner

go-jet commented Oct 9, 2024

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

@safaci2000
Copy link
Contributor Author

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
Copy link
Owner

go-jet commented Oct 10, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

go-jet commented Oct 11, 2024

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
}

@go-jet go-jet added the good first issue Good for newcomers label Oct 11, 2024
@safaci2000
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants