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

Fixed the construction of query for tables with schema #26

Merged
merged 4 commits into from
Apr 11, 2022
Merged

Fixed the construction of query for tables with schema #26

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56e8dba
Fixed the constuction of query for tables with schema
talkanbaev-artur Apr 9, 2022
e33c2b2
Minor fixes to the cache calls of sql query buffer
talkanbaev-artur Apr 10, 2022
daddf7c
Prioritised checks for simple queries in buffer
talkanbaev-artur Apr 10, 2022
0cfb3e6
Added select all stmts to chaching in buffer
talkanbaev-artur Apr 11, 2022
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
37 changes: 26 additions & 11 deletions builder/buffer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,33 @@ func (b *Buffer) WriteEscape(value string) {
}

func (b Buffer) escape(table, value string) string {
if value == "*" {
if table == "" {
return value
}
return b.Quoter.ID(table) + ".*"
}

key := escapeCacheKey{table: table, value: value, quoter: b.Quoter}
escapedValue, ok := escapeCache.Load(key)
if ok {
return escapedValue.(string)
}

var escaped_table string
if table != "" {
if 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 {
escaped_table = b.Quoter.ID(table)
}
}

if value == "*" {
Fs02 marked this conversation as resolved.
Show resolved Hide resolved
if table == "" {
return value
}
return escaped_table + ".*"
Copy link
Member

Choose a reason for hiding this comment

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

hmm, maybe we should this field to escapeCache as well, so escape_table doesn't get recomputed everytime

Copy link
Contributor Author

@talkanbaev-artur talkanbaev-artur Apr 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I think we store it already.

sql/builder/buffer.go

Lines 162 to 169 in 56e8dba

result := strings.Join(parts, ".")
if len(parts) == 1 && table != "" {
result = escaped_table + "." + result
}
escapedValue = result
}
escapeCache.Store(key, escapedValue)

So I just changed the order for the cache check. If it is done before the computation of the escaped_table, then the key of table/value/quoter reuses the result of escaped value.

Or did you mean that it would be beneficial just to have a separate cache for table names?

Copy link
Member

Choose a reason for hiding this comment

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

So I just changed the order for the cache check

this is what I mean

Copy link
Member

Choose a reason for hiding this comment

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

btw, above line should call escapeCache.Store(key, escapedValue) first before returning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right... I think it does call the cache store before the return

Copy link
Member

Choose a reason for hiding this comment

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

looks like it still not updated to store cache before returning? so actually the cache will never be stored

also, I think we can merge this if with below if?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sorry - I misunderstood you. I merged statements, and know "table.*" are cached too

}

if len(value) > 0 && value[0] == UnescapeCharacter {
escapedValue = value[1:]
} else if _, err := strconv.Atoi(value); err == nil {
Expand All @@ -138,17 +152,18 @@ func (b Buffer) escape(table, value string) string {
escapedValue = value[:start+1] + b.escape(table, value[start+1:end]) + value[end:]
} else {
parts := strings.Split(value, ".")
if len(parts) == 1 && table != "" {
parts = []string{table, parts[0]}
}
for i, part := range parts {
part = strings.TrimSpace(part)
if part == "*" && i == len(parts)-1 {
break
}
parts[i] = b.Quoter.ID(part)
}
escapedValue = strings.Join(parts, ".")
result := strings.Join(parts, ".")
if len(parts) == 1 && table != "" {
result = escaped_table + "." + result
}
escapedValue = result
}

escapeCache.Store(key, escapedValue)
Expand Down
5 changes: 5 additions & 0 deletions builder/buffer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func TestBuffer_escape(t *testing.T) {
field: "*",
result: "[user].*",
},
{
table: "user.user",
field: "*",
result: "[user].[user].*",
},
{
table: "user",
field: "address as home_address",
Expand Down