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

go/adbc/driver/snowflake: GetObjects API inconsistent case sensitivity for patterns #1314

Closed
ryan-syed opened this issue Nov 21, 2023 · 7 comments · Fixed by #1328
Closed

Comments

@ryan-syed
Copy link
Contributor

The spec doesn't seem clear for case-sensitivity.

Similarly, in the getObjectsDbSchemas driver implementation it uses LIKE whereas getObjectsTables uses ILIKE. What should be the correct handling as per the spec? Should we allow the patterns to be case-insensitive?

func (c *cnxn) getObjectsDbSchemas(ctx context.Context, depth adbc.ObjectDepth, catalog *string, dbSchema *string) (result map[string][]string, err error) {
	if depth == adbc.ObjectDepthCatalogs {
		return
	}

	conditions := make([]string, 0)
	if catalog != nil && *catalog != "" {
		conditions = append(conditions, ` CATALOG_NAME LIKE \'`+*catalog+`\'`)
	}
	if dbSchema != nil && *dbSchema != "" {
		conditions = append(conditions, ` SCHEMA_NAME LIKE \'`+*dbSchema+`\'`)
	}

	cond := strings.Join(conditions, " AND ")
	if cond != "" {
		cond = `statement := 'SELECT * FROM (' || statement || ') WHERE ` + cond + `';`
	}
func (c *cnxn) getObjectsTables(ctx context.Context, depth adbc.ObjectDepth, catalog *string, dbSchema *string, tableName *string, columnName *string, tableType []string) (result internal.SchemaToTableInfo, err error) {
	if depth == adbc.ObjectDepthCatalogs || depth == adbc.ObjectDepthDBSchemas {
		return
	}

	result = make(internal.SchemaToTableInfo)
	includeSchema := depth == adbc.ObjectDepthAll || depth == adbc.ObjectDepthColumns

	conditions := make([]string, 0)
	if catalog != nil && *catalog != "" {
		conditions = append(conditions, ` TABLE_CATALOG ILIKE \'`+*catalog+`\'`)
	}
	if dbSchema != nil && *dbSchema != "" {
		conditions = append(conditions, ` TABLE_SCHEMA ILIKE \'`+*dbSchema+`\'`)
	}
	if tableName != nil && *tableName != "" {
		conditions = append(conditions, ` TABLE_NAME ILIKE \'`+*tableName+`\'`)
	}
@ryan-syed
Copy link
Contributor Author

#1299 Found while working on adding more metadata tests

@lidavidm
Copy link
Member

The behavior is up to the vendor; Snowflake's case-sensitivity is odd so it may be best to be case-insensitive by default

@zeroshade
Copy link
Member

In my experience, most SQL dialects tend to be case-insensitive by default unless quotes are used to enforce the casing, so I agree with @lidavidm here. As long as we support allowing the user to include quotes in the value so that they can choose to enforce casing if they want, I think we're okay.

@ryan-syed
Copy link
Contributor Author

Thanks, I will work on making the GetObjects case-insensitive for the patterns. Should I create an issue for BigQuery driver too, as it uses LIKE throughout in its implementation:

query = string.Concat(query, string.Format(" WHERE table_name LIKE '{0}'", Sanitize(tableNamePattern)));

In my experience, most SQL dialects tend to be case-insensitive by default unless quotes are used to enforce the casing, so I agree with @lidavidm here. As long as we support allowing the user to include quotes in the value so that they can choose to enforce casing if they want, I think we're okay.

The behavior is up to the vendor; Snowflake's case-sensitivity is odd so it may be best to be case-insensitive by default

@lidavidm
Copy link
Member

Yes, please go ahead and file another issue - thanks!

@ryan-syed
Copy link
Contributor Author

Yes, please go ahead and file another issue - thanks!

created issue for BigQuery here: #1321

@ryan-syed
Copy link
Contributor Author

Created PR: #1328

@lidavidm lidavidm added this to the ADBC Libraries 0.9.0 milestone Nov 29, 2023
CurtHagenlocher pushed a commit that referenced this issue Dec 4, 2023
### Description:
`GetObjects` API was inconsistent case sensitivity for patterns.
`getObjectsDbSchemas` driver implementation used `LIKE` whereas
`getObjectsTables` used `ILIKE`

### Solution:
Based on discussion here:
#1314 changed `GetObjects`
API to use `ILIKE` throughout instead of `LIKE`

### Testing:
Added tests for lowercase, uppercase and `_` wildcard

Fixes #1314.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants