Invalid name generation when using python reserved keywords by QuentinN42 · Pull Request #89 · sqlc-dev/sqlc-gen-python · GitHub

Fixes #88

To fix this issue, here is my proposition:

 class Author(pydantic.BaseModel):
+     model_config = pydantic.ConfigDict(
+        validate_by_alias=True,
+        validate_by_name=True,
+    )
     id: int
+    class_: str = pydantic.Field(
+        alias="class",
+    )
-    class: str

I'm replacing the class reserved keyword by class_.
I add validate_by_name to allow us to define the class using Author(class_=...).
I add validate_by_alias to allow JSON serde.

Changes overview

I've created a new file : internal/poet/reserved.go to append _ to reserved names.
I've modified internal/gen.go::pydanticNode to add model_config = pydantic.ConfigDict
I've modified internal/gen.go::fieldNode:

If not reserved keyword, code unchanged.
Else if using pydantic, add the _ and add pydantic.Field(...)
Else if dataclasses, only add the _

Added tests

Questions / possible issues

To run the tests, I just clonned the repo and ran make test is it correct ?
I had to update all comment from sqlc v1.28.0 to sqlc v1.29.0 is this intended ?
The wasm sha256 changed, should I commit that ?
In internal/poet/reserved.go I hardcoded reservedKeywords we should double check it and check if we want to keep it like that.
Do you want me to add more tests ?

…n reserved keywords Signed-off-by: QuentinN42 <quentin@lieumont.fr>

Signed-off-by: QuentinN42 <quentin@lieumont.fr>

…figDict Signed-off-by: QuentinN42 <quentin@lieumont.fr>

Signed-off-by: QuentinN42 <quentin@lieumont.fr>

Pull request overview

This PR addresses the handling of Python reserved keywords in generated code by transforming field names that conflict with keywords (e.g., class → class_) and adding appropriate Pydantic configuration for serialization/deserialization.

Key Changes:

Introduced reserved keyword detection and field name transformation logic (internal/poet/reserved.go)
Modified Pydantic models to include model_config with validate_by_alias=True and validate_by_name=True
Updated field generation to handle reserved keywords differently for Pydantic (with alias) vs dataclasses (simple rename)

Reviewed changes

Copilot reviewed 50 out of 52 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/poet/reserved.go	New file with reserved keyword list and transformation functions
internal/poet/reserved_test.go	Tests for reserved keyword detection and transformation
internal/poet/builders.go	Modified Name() builder to apply field name transformation
internal/gen.go	Updated pydanticNode() to add model_config and fieldNode() to handle reserved keywords
internal/gen_test.go	New unit tests for field node generation with reserved keywords
internal/endtoend/endtoend_test.go	Modified error message output in test
internal/endtoend/testdata/emit_pydantic_models_with_reserved_keywords/*	New end-to-end test for Pydantic with reserved keywords
internal/endtoend/testdata/dataclasses_with_reserved_keywords/*	New end-to-end test for dataclasses with reserved keywords
internal/endtoend/testdata/emit_pydantic_models/db/models.py	Updated to include model_config in all Pydantic models
internal/endtoend/testdata/*/sqlc.yaml	Updated WASM SHA256 checksums
internal/endtoend/testdata//python/.py	Updated version comments from v1.28.0 to v1.29.0
examples/src//.py	Updated version comments from v1.28.0 to v1.29.0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The model_config with validate_by_alias=True and validate_by_name=True is being added to ALL Pydantic models, even those without reserved keywords. This adds unnecessary validation overhead for models that don't need it. Consider only adding this configuration when the model actually contains fields with reserved keyword names, or at minimum make it conditional based on whether any field in the model uses reserved keywords.

Critical bug: Applying FieldName(id) to ALL uses of poet.Name() will break code generation. The Name function is used for many purposes beyond field names, including:

Built-in constants like "True", "False", "None" (lines 629, 633 in gen.go)
Module names like "dataclasses", "enum" (lines 576, 782, 787)
Type names like "str", "int" (lines 781, 1052, 1145)
Variable names like "self", "conn", "row", "result" (lines 866, 868, 1007, 1031, etc.)

Once the reserved keywords list includes "True", "False", "None", "for", "return", "yield", "is", "not", "in", "or", "and", "as", "with", "if", "else", etc., this will transform all these identifiers incorrectly. This change should be reverted, and FieldName() should only be applied in specific contexts where field names are being generated.

Typo in function name: "ShouldUdateReservedKeywords" should be "ShouldUpdateReservedKeywords"

The reserved keywords list is incomplete. Python 3.x has 35 keywords, but only 17 are included here. Missing keywords include: False, None, True, async, await, continue, def, del, from, global, import, lambda, nonlocal, pass, while, and match/case (Python 3.10+). This could cause syntax errors when database columns use these names.

This change breaks the test logic. The error message should show diff (the actual difference between want and got) to help debug test failures, not got (the raw output). This makes it harder to understand what went wrong when tests fail.

Suggested change

      
            				t.Errorf("sqlc diff mismatch (-want +got):\n%s", got)
          
            				t.Errorf("sqlc diff mismatch (-want +got):\n%s", diff)

QuentinN42 added 5 commits July 25, 2025 15:37

feat: added invalid test for invalid name generation when using pytho…

19e8d56

…n reserved keywords Signed-off-by: QuentinN42 <quentin@lieumont.fr>

fix: added reserved python keywords

f928244

Signed-off-by: QuentinN42 <quentin@lieumont.fr>

refactor: use poet.Node

24fcd39

Signed-off-by: QuentinN42 <quentin@lieumont.fr>

test: added more tests to the poet pkg and generated the pydantic.Con…

a5d1ee8

…figDict Signed-off-by: QuentinN42 <quentin@lieumont.fr>

feat: added dataclasses tests

068fd25

Signed-off-by: QuentinN42 <quentin@lieumont.fr>

devstein requested review from Copilot, devstein and kyleconroy and removed request for kyleconroy November 24, 2025 16:17

Copilot started reviewing on behalf of devstein November 24, 2025 16:18 View session

Copilot finished reviewing on behalf of devstein November 24, 2025 16:20

Copilot AI reviewed Nov 24, 2025

View reviewed changes

This was referenced Feb 19, 2026

Invalid name generation when using python reserved keywords enveritas/sqlc-gen-python#3

Merged

Invalid name generation when using python reserved keywords #96

Closed

Invalid name generation when using python reserved keywords enveritas/sqlc-gen-python#4

Merged

+              		Body: poet.Nodes(
+              			&pyast.Assign{
+              				Targets: []*pyast.Node{
+              					{
+              						Node: &pyast.Node_Name{
+              							Name: &pyast.Name{Id: "model_config"},
+              						},
+              					},
+              				},
+              				Value: poet.Node(
+              					&pyast.Call{
+              						Func: &pyast.Node{
+              							Node: &pyast.Node_Attribute{
+              								Attribute: &pyast.Attribute{
+              									Value: &pyast.Node{
+              										Node: &pyast.Node_Name{
+              											Name: &pyast.Name{
+              												Id: "pydantic",
+              											},
+              										},
+              									},
+              									Attr: "ConfigDict",
+              								},
+              							},
+              						},
+              						Keywords: []*pyast.Keyword{
+              							{
+              								Arg:   "validate_by_alias",
+              								Value: poet.Name("True"),
+              							},
+              							{
+              								Arg:   "validate_by_name",
+              								Value: poet.Name("True"),
+              							},
+              						},
+              					},
+              				),
+              			},
+              		),

               	return &ast.Node{
               		Node: &ast.Node_Name{
-              			Name: &ast.Name{Id: id},
+              			Name: &ast.Name{Id: FieldName(id)},

Invalid name generation when using python reserved keywords by QuentinN42 · Pull Request #89 · sqlc-dev/sqlc-gen-python · GitHub

Uh oh!

Invalid name generation when using python reserved keywords - #89

Invalid name generation when using python reserved keywords#89
QuentinN42 wants to merge 5 commits into
sqlc-dev:mainfrom
Escape-Technologies:main

QuentinN42 commented Jul 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 24, 2025

Uh oh!

Copilot AI Nov 24, 2025

Uh oh!

Copilot AI Nov 24, 2025

Uh oh!

Copilot AI Nov 24, 2025

Uh oh!

Copilot AI Nov 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

+              	}
+              }
+              func Test_FieldName_ShouldUdateReservedKeywords(t *testing.T) {

+              var reservedKeywords = []string{
+              	"class",
+              	"if",
+              	"else",
+              	"elif",
+              	"not",
+              	"for",
+              	"and",
+              	"in",
+              	"is",
+              	"or",
+              	"with",
+              	"as",
+              	"assert",
+              	"break",
+              	"except",
+              	"finally",
+              	"try",
+              	"raise",
+              	"return",
+              	"yield",
+              }

	t.Errorf("sqlc diff mismatch (-want +got):\n%s", got)
	t.Errorf("sqlc diff mismatch (-want +got):\n%s", diff)

Invalid name generation when using python reserved keywords by QuentinN42 · Pull Request #89 · sqlc-dev/sqlc-gen-python · GitHub

Uh oh!

Invalid name generation when using python reserved keywords - #89

Invalid name generation when using python reserved keywords#89QuentinN42 wants to merge 5 commits intosqlc-dev:mainsqlc-dev/sqlc-gen-python:mainfrom Escape-Technologies:mainEscape-Technologies/sqlc-gen-python:mainCopy head branch name to clipboard

Conversation

QuentinN42 commented Jul 25, 2025

Changes overview

Questions / possible issues

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Invalid name generation when using python reserved keywords#89
QuentinN42 wants to merge 5 commits into
sqlc-dev:mainfrom
Escape-Technologies:main