-
Notifications
You must be signed in to change notification settings - Fork 73
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
Override view name in querier #167
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## v1-stable #167 +/- ##
=============================================
+ Coverage 68.4% 68.58% +0.18%
=============================================
Files 19 19
Lines 1557 1566 +9
=============================================
+ Hits 1065 1074 +9
Misses 442 442
Partials 50 50
Continue to review full report at Codecov.
|
func (q *Querier) WithView(viewName string) *Querier { | ||
newQ := newQuerier(q.dbtx, q.Dialect, q.Logger) | ||
newQ.viewName = viewName | ||
return newQ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are losing tag here.
querier.go
Outdated
// QualifiedView returns quoted qualified view name. | ||
func (q *Querier) QualifiedView(view View) string { | ||
v := q.QuoteIdentifier(view.Name()) | ||
if q.viewName != "" { | ||
v = q.QuoteIdentifier(q.viewName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh. view
parameter is ignored when q.viewName
is set.
reform-db/cmd_init_test.go
Outdated
@@ -38,7 +38,11 @@ func (s *ReformDBSuite) TestInit() { | |||
|
|||
fis, err := ioutil.ReadDir(dir) | |||
s.Require().NoError(err) | |||
s.Require().Len(fis, 4) | |||
if s.db.Dialect == sqlite3.Dialect { | |||
s.Require().Len(fis, 4) // generate 4 struct files for sqlite3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use views for SQLite3 too.
@bitxel May you please check this branch? It should work for all |
from = q.qualifiedViewName + " AS " + from | ||
} | ||
|
||
return fmt.Sprintf("%s %s FROM %s %s", query, strings.Join(q.QualifiedColumns(view), ", "), from, tail) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we only change table name in FROM
clause and alias it to the old name. That should do the trick for all SELECT
s. @bitxel, please check it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understand, it's a neat way that only change the view name when making SQL,
but it may lose the schema name, so it's caller's responsibility to add and quote the schema name
751abbf
to
27faa85
Compare
# Conflicts: # querier.go # querier_test.go
// QualifiedView returns quoted qualified view name. | ||
// WithQualifiedViewName returns a copy of Querier with set qualified view name. | ||
// Returned Querier is tied to the same DB or TX. | ||
// TODO Support INSERT/UPDATE/DELETE. More test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Optional linters] reported by reviewdog 🐶
querier.go:73: Line contains TODO/BUG/FIXME: "TODO Support INSERT/UPDATE/DELETE. More ..." (godox)
@@ -85,6 +85,20 @@ func ExampleQuerier_WithContext() { | |||
_, _ = DB.WithContext(ctx).SelectAllFrom(ProjectTable, "") | |||
} | |||
|
|||
func ExampleQuerier_WithQualifiedViewName() { | |||
_, err := DB.WithQualifiedViewName("people_0").FindByPrimaryKeyFrom(PersonTable, 1) | |||
if err != reform.ErrNoRows { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Optional linters] reported by reviewdog 🐶
err113: do not compare errors directly, use errors.Is() instead: "err != reform.ErrNoRows" (goerr113)
s.Equal(`"people"`, s.q.QualifiedView(PersonTable)) | ||
s.Equal(`"people"`, s.q.WithQualifiedViewName("ignored").QualifiedView(PersonTable)) | ||
|
||
case mssql.Dialect, sqlserver.Dialect: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Optional linters] reported by reviewdog 🐶
SA1019: mssql.Dialect is deprecated: Use sqlserver.Dialect instead. https://github.com/denisenkom/go-mssqldb#deprecated (staticcheck)
By @bitxel.
Closes #154.
TODO
INSERT
/UPDATE
/DELETE
.