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

fixed panic error when null collection received #33

Closed
wants to merge 3 commits into from

Conversation

yesoreyeram
Copy link
Contributor

@yesoreyeram yesoreyeram commented Sep 8, 2022

  • Fix panic when null collection received
  • Added Format selector in the front end
  • Defaults results to table format
  • Remove automatic timeseries formatting
  • Fixed a bug where hiding query doesn't honoured

Copy link
Contributor

@scottlepp scottlepp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems we are adding a new feature here, not just fixing the bug. We should really try keep these as separate prs.

"Remove automatic timeseries format". This is the convention in clickhouse, sqlyze, databricks and other sql plugins. The code here was from sqlds. I think we should be consistent. Is there a reason we did that?

So we need to revert that change, or update the read me.

@yesoreyeram
Copy link
Contributor Author

makes sense to have separate PR. Closing this and will open a PR just for the panic. ( but still not convinced that we should remove the format selector. we can discuss that over #34 ).

@yesoreyeram yesoreyeram closed this Sep 9, 2022
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.

None yet

2 participants