feat: support dialect-aware query conditions by atzoum · Pull Request #180 · rudderlabs/sqlconnect-go

atzoum · 2024-09-09T11:53:28Z

Description

Introducing the following methods.

// QueryCondition returns a dialect-specific query condition sql string for the provided identifier, operator and value(s).
//
// E.g. QueryCondition("age", "gt", 18) returns "age > 18"
//
// Each operator has a different number of arguments, e.g. [eq] requires one argument, [in] requires at least one argument, etc.
// See [op] package for the list of supported operators
QueryCondition(identifier, operator string, args ...any) (sql string, err error)

// TimestampAdd returns an expression that adds the interval to the timestamp value.
// The value can either be a string literal (column, timestamp, function etc.) or a [time.Time] value.
TimestampAdd(timeValue any, interval int, unit string) (Expression, error)

// DateAdd returns an expression that adds the interval to the date value.
// The value can either be a string literal (column, timestamp, function etc.) or a [time.Time] value.
// Values are cast to [DATE].
DateAdd(dateValue any, interval int, unit string) (Expression, error)

Using goqu internally which:

Provides proper go type(s) recognition in parameters (int, float, string, time.Time, etc.)
It properly escapes special characters from string values (')
It handles differences in warehouse dialects seamlessly eg. bool_col IS TRUE vs bool_col = 'TRUE'.

Linear Ticket

resolves PRO-3401

Security

The code changed/added as part of this pull request won't create any security issues with how the software is being used.

codecov · 2024-09-09T11:57:36Z

Codecov Report

Attention: Patch coverage is 96.31068% with 19 lines in your changes missing coverage. Please review.

Project coverage is 87.04%. Comparing base (8cab1fd) to head (da28b2b).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sqlconnect/internal/base/goqu_dialect.go	89.14%	13 Missing and 1 partial ⚠️
sqlconnect/op/operator.go	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
+ Coverage   85.58%   87.04%   +1.46%     
==========================================
  Files          78       87       +9     
  Lines        3343     3851     +508     
==========================================
+ Hits         2861     3352     +491     
- Misses        328      345      +17     
  Partials      154      154

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shubhammehra4

LGTM

sprksh · 2024-09-12T07:22:38Z

sqlconnect/op/operator.go

+	// v < left OR v > right
+	NotBetween Operator = "notbetween"
+	// left >= now() - INTERVAL right
+	NbfInterval Operator = "nbfinterval"


The key nbfinterval is not clear to me. Can we give it a clearer name?

Open to suggestions! nbf prefix stands for not before.

Instead can we have something like
beforeintervalbounded & beforeintervalunbounded
afterintervalbounded & afterintervalunbounded

Also, we can use camelCase or snake case operators. Will make things much clearer. Especially in case we are using bigger ones.

imo operators should be kept small and case insensitive

what about:
gti --> gt an interval
gtei --> gte an interval
lti --> less than an interval
ltei --> less than an interval
betweeni --> between an interval

Another option would be to keep using gt, gte, lt, lte, between, etc. and allow a special value format for intervals, e.g. "now-1d"

finally, we can just keep ti simple for now and name it inlast

sprksh

Went through the code and tests. Looks great.

sprksh · 2024-09-12T07:48:01Z

sqlconnect/op/operator.go

+	// NOT LIKE
+	NotLike Operator = "notlike"
+	// left <= v <= right
+	Between Operator = "between"


For Between, Should we think of some way to specify left and right bounded/unbounded conditions.

Also NotBetween

🤖 I have created a release *beep* *boop* --- ## [1.10.0](v1.9.0...v1.10.0) (2024-09-12) ### Features * **databricks:** bump github.com/databricks/databricks-sql-go from 1.6.0 to 1.6.1 ([#174](#174)) ([8cab1fd](8cab1fd)) * **snowflake:** bump github.com/snowflakedb/gosnowflake from 1.10.1 to 1.11.1 ([#171](#171)) ([e64ba77](e64ba77)) * support dialect-aware query conditions ([#180](#180)) ([08906ee](08906ee)) ### Miscellaneous * all warehouses support views in ListTables ([#169](#169)) ([098a378](098a378)) * **deps:** bump github.com/aws/aws-sdk-go-v2/credentials from 1.17.27 to 1.17.32 ([#179](#179)) ([47afb2e](47afb2e)) * **deps:** bump github.com/rudderlabs/rudder-go-kit from 0.36.2 to 0.40.0 ([#178](#178)) ([a4ee52e](a4ee52e)) * **deps:** bump google.golang.org/api from 0.194.0 to 0.196.0 ([#177](#177)) ([aaee20d](aaee20d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

atzoum force-pushed the feat.filterCondition branch 3 times, most recently from 7a1a31d to cebc595 Compare September 10, 2024 10:38

atzoum changed the title ~~[WIP] feat: support dialect-aware query conditions~~ feat: support dialect-aware query conditions Sep 10, 2024

feat: support dialect-aware query conditions

6854d81

atzoum force-pushed the feat.filterCondition branch from cebc595 to 6854d81 Compare September 10, 2024 12:02

fixup! feat: support dialect-aware query conditions

d8671ae

atzoum requested review from shubhammehra4 and sprksh September 10, 2024 14:15

atzoum marked this pull request as ready for review September 10, 2024 14:17

atzoum requested review from AchuthaSourabhC and RanjeetMishra as code owners September 10, 2024 14:17

atzoum requested a review from kuldeep0020 September 11, 2024 08:36

shubhammehra4 approved these changes Sep 11, 2024

View reviewed changes

chore: add nbfinterval operation

2308aef

atzoum force-pushed the feat.filterCondition branch from 588fd0a to 2308aef Compare September 12, 2024 07:09

sprksh reviewed Sep 12, 2024

View reviewed changes

sprksh approved these changes Sep 12, 2024

View reviewed changes

sprksh reviewed Sep 12, 2024

View reviewed changes

chore: operator (short) names

da28b2b

atzoum merged commit 08906ee into main Sep 12, 2024

atzoum deleted the feat.filterCondition branch September 12, 2024 10:26

devops-github-rudderstack mentioned this pull request Sep 12, 2024

chore: release 1.10.0 #170

Merged

Conversation

atzoum commented Sep 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Linear Ticket

Security

Uh oh!

codecov bot commented Sep 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shubhammehra4 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atzoum Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sprksh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

atzoum commented Sep 9, 2024 •

edited

Loading

codecov bot commented Sep 9, 2024 •

edited

Loading

atzoum Sep 12, 2024 •

edited

Loading