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

Dynamic query #2343

Closed
wants to merge 6 commits into from
Closed

Conversation

jwc-clinnection
Copy link
Contributor

Implemented dynamic query as suggested by go-aegian in issues:

Support dynamic where clauses #2060
Support dynamic order by clause #2061

Given the desire to move away from templates, I suspect that this would be a temporary fix but a much desired one.

@jwc-clinnection
Copy link
Contributor Author

It is failing some tests. For example this is an expected change given the pull request:

                -               rows, err := q.db.Query(ctx, joinTextArray)
                +               sql := joinTextArray
                +       
                +               rows, err := q.db.Query(ctx, sql)

I tried:

make build-endtoend
cd ./internal/endtoend/testdata && go build ./...

But, I still get errors when I run:

make test

Should I be doing something to regenerate the test samples? If so, may I have some guidance on the procedure?

@jwc-clinnection
Copy link
Contributor Author

We have discovered a bug in this pull request. If "query_parameter_limit: 0" is not set, then it fails to generate the correct filter code. We are working to to resolve this problem.

@jwc-clinnection
Copy link
Contributor Author

We have corrected the issue, and it now generates correct code for all cases of query_parameter_limit

@go-aegian
Copy link

@jwc-clinnection have you got any advice from @kyleconroy on how to move this forward to the release?

In the meantime I did manage a workaround process until your work is released though.

@andrewmbenton
Copy link
Collaborator

There is still an active discussion regarding sqlc's path forward for dynamic SQL, so I'm closing this for now until that discussion is resolved. Follow along with issues #2061 and #2060 or this discussion: #364

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 this pull request may close these issues.

3 participants