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.

Sign up for GitHub

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

Jump to bottom

Alias issue #70

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Alias issue #70

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d596c12
Alias issue #69
Apr 16, 2024
6f457cc
Revert changes for Select
fairking Apr 18, 2024
7ea04ad
Alias issue
fairking Apr 18, 2024
e498e85
Alias issue
fairking Apr 18, 2024
512c4ae
Alias issue #70 (update escapeSchema method name)
fairking Apr 19, 2024
82fb599
Alias issue (remove non as aliases)
fairking Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions builder/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,22 @@ func (b Buffer) escape(table, value string) string {

var escaped_table string
if table != "" {
if i := strings.Index(strings.ToLower(table), " as "); i > -1 {
return b.escape(table[:i], "") + " AS " + b.Quoter.ID(table[i+4:])
}
if b.AllowTableSchema && strings.IndexByte(table, '.') >= 0 {
parts := strings.Split(table, ".")
for i, part := range parts {
part = strings.TrimSpace(part)
parts[i] = b.Quoter.ID(part)
}
escaped_table = strings.Join(parts, ".")
} else if i := strings.Index(strings.ToLower(table), " as "); i > -1 {
escaped_table = b.escape(table[:i], "") + " AS " + b.Quoter.ID(table[i+4:])
} else {
escaped_table = b.Quoter.ID(strings.ReplaceAll(table, ".", "_"))
parts := strings.Split(table, " ")
for i, part := range parts {
part = strings.TrimSpace(part)
parts[i] = b.Quoter.ID(part)
}
escaped_table = strings.Join(parts, " ")
}
}

Expand Down
32 changes: 32 additions & 0 deletions builder/query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,22 @@ func TestQuery_Build(t *testing.T) {
result: "SELECT `users`.* FROM `users` FOR UPDATE;",
query: rel.From("users").Lock("FOR UPDATE"),
},
{
result: "SELECT `c`.`id`,`c`.`name` FROM `contacts` `c`;",
query: rel.Select("c.id", "c.name").From("contacts c"),
},
{
result: "SELECT `c`.`id`,`c`.`name` FROM `contacts` AS `c`;",
query: rel.Select("c.id", "c.name").From("contacts as c"),
},
{
result: "SELECT `c`.`id`,`c`.`name` FROM `contacts` `c`;",
query: rel.Select("c.id").Select("c.name").From("contacts c"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why not using single Select function? seems this change is also not related to the alias fix?

Copy link
Author

@fairking fairking Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that is not related to aliases things, but I was thinking it would be beneficial and more convenient for everyone if we bring this small improvement as well.

My issue was to include some extra columns based on the condition, and it was more intuitive. I thought something else was broken, not my code when I was using it:

var query = rel.Select("id", "title").From("pools")

if includeAvgScore {
    query.Select("avgScore")
} else {
    query.Select("score")
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please feel free to make any changes. That was my opinion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, then can you create separate PR for this?

Copy link
Author

@fairking fairking Apr 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, Now we have alias changes only.

},
{
result: "SELECT `c`.`id`,`c`.`name` FROM `contacts` AS `c`;",
query: rel.Select("c.id").Select("c.name").From("contacts as c"),
},
}

for _, test := range tests {
Expand Down Expand Up @@ -185,6 +201,22 @@ func TestQuery_Build_ordinal(t *testing.T) {
result: "SELECT \"users\".* FROM \"users\" FOR UPDATE;",
query: rel.From("users").Lock("FOR UPDATE"),
},
{
result: "SELECT \"c\".\"id\",\"c\".\"name\" FROM \"contacts\" \"c\";",
query: rel.Select("c.id", "c.name").From("contacts c"),
},
{
result: "SELECT \"c\".\"id\",\"c\".\"name\" FROM \"contacts\" AS \"c\";",
query: rel.Select("c.id", "c.name").From("contacts as c"),
},
{
result: "SELECT \"c\".\"id\",\"c\".\"name\" FROM \"contacts\" \"c\";",
query: rel.Select("c.id").Select("c.name").From("contacts c"),
},
{
result: "SELECT \"c\".\"id\",\"c\".\"name\" FROM \"contacts\" AS \"c\";",
query: rel.Select("c.id").Select("c.name").From("contacts as c"),
},
}

for _, test := range tests {
Expand Down
Loading