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

WIP: Support new dbplyr.trace option #930

Closed
wants to merge 2 commits into from
Closed

WIP: Support new dbplyr.trace option #930

wants to merge 2 commits into from

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Jul 4, 2022

Print SQL before querying or executing, and a corresponding message when done.

Closes #827.

@mgirlich
Copy link
Collaborator

@krlmlr Do you want to continue with this? 😄

@krlmlr
Copy link
Member Author

krlmlr commented Jul 10, 2023

What do you want this to look like?

@mgirlich
Copy link
Collaborator

I'm basically quite happy with the approach already. As this was your idea: is there anything else you want/need?

What's missing from this PR in my opinion is:

  • NEWS
  • It might make sense to add a better visual indicator between queries. In the following example it is a bit too difficult to quickly distinguish the two queries:
[17]: dbExecute()
ANALYZE `dbplyr_011`
[17]: dbExecute() done
[18]: dbExecute()
INSERT INTO `dbplyr_010` (`x`, `y`)
SELECT *
FROM (
  SELECT *
  FROM `dbplyr_011`
) AS `...y`
WHERE NOT EXISTS (
  SELECT 1 FROM `dbplyr_010`
  WHERE (`dbplyr_010`.`x` = `...y`.`x`)
)
[18]: dbExecute() done

@hadley
Copy link
Member

hadley commented Dec 21, 2023

Closing, since if you're still interested, you'd now want to implement on top of db_execute()/db_get_query().

@hadley hadley closed this Dec 21, 2023
@krlmlr
Copy link
Member Author

krlmlr commented Dec 21, 2023

I'm confused. Are db_execute() and db_get_query() becoming generics? They don't seem to be in 267a12e .

@hadley
Copy link
Member

hadley commented Dec 21, 2023

Why would they need to be generics?

@krlmlr
Copy link
Member Author

krlmlr commented Dec 21, 2023

If they are not, then this may mean that you're asking me to reimplement this PR using new idioms. I'd be happy to review a PR instead.

@hadley
Copy link
Member

hadley commented Dec 21, 2023

I'm not sure this feature is necessary any more, now that errors will include the generated SQL.

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.

Debug mode
3 participants