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

Performance: Add links to downsampling tutorials #45

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

amotl
Copy link
Member

@amotl amotl commented Feb 26, 2024

About

Add two more references to tutorials on the community forum. Thanks, @hlcianfagna and @hammerhead.

  • Performance: Add link to "Resampling time-series data with DATE_BIN"
  • Performance: Add link to "Advanced downsampling with the LTTB algorithm"

Preview

/cc @surister, @karynzv, @marijaselakovic, @seut, @wierdvanderhaar

@amotl amotl added cross linking Linking to different locations of the documentation. new content New content being added. labels Feb 26, 2024
@amotl amotl force-pushed the amo/performance-downsampling branch from 4ce425b to ecc7cb4 Compare February 26, 2024 22:43
@amotl amotl marked this pull request as ready for review February 26, 2024 22:44
@amotl amotl force-pushed the amo/performance-downsampling branch from ecc7cb4 to aa86952 Compare February 26, 2024 22:50
Comment on lines 161 to 167
WITH downsampleddata AS
( SELECT lttb_with_parallel_arrays(
array(SELECT n FROM demo ORDER BY n),
array(SELECT reading FROM demo ORDER BY n)
,100) AS lttb)
SELECT unnest(lttb['0']) as n, unnest(lttb['1']) AS reading
FROM downsampleddata;
Copy link
Member Author

@amotl amotl Feb 27, 2024

Choose a reason for hiding this comment

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

@hammerhead: This SQL statement probably also has a formatting hiccup, like #41 (review)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we finally employ a proper SQL linter / formatter for all the technical writing endeavours?

Copy link
Member Author

Choose a reason for hiding this comment

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

Reformatted using https://sqlformat.org/, then edited manually a bit. Is it a good choice?

Suggested change
WITH downsampleddata AS
( SELECT lttb_with_parallel_arrays(
array(SELECT n FROM demo ORDER BY n),
array(SELECT reading FROM demo ORDER BY n)
,100) AS lttb)
SELECT unnest(lttb['0']) as n, unnest(lttb['1']) AS reading
FROM downsampleddata;
WITH downsampleddata AS
(SELECT lttb_with_parallel_arrays(
array(SELECT n FROM demo ORDER BY n),
array(SELECT reading FROM demo ORDER BY n), 100) AS lttb)
SELECT unnest(lttb['0']) AS n,
unnest(lttb['1']) AS reading
FROM downsampleddata;

Copy link
Member

Choose a reason for hiding this comment

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

I would love to have a standard formatting tool that we can agree on. Like (at least in our team) we settled on black for Python, and it's considered the truth™️ that we all stick to. Without the "then edited manually a bit" part you mentioned, as that kind of defeats the purpose.

For community posts, I don't dare to be the one nit-picking on formatting all the time, but as soon as we are talking about the main documentation, there needs to be a consistent style and quality of formatting. With formatting being such a subjective topic, only a tool we all abide to can do that job.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree we need to level up. Future improvements, also expanding into cratedb-examples, will/should/may probably include:

=> We probably want to break this out into a different discussion.

Copy link
Member Author

@amotl amotl Feb 27, 2024

Choose a reason for hiding this comment

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

I've diverted this into another issue. Thanks.

@amotl
Copy link
Member Author

amotl commented Feb 27, 2024

Thanks for approving. Now that GH-42 is also approved, we only need approval for GH-41, which is at the bottom of the stack, and needs to be processed first. Thanks!

Base automatically changed from amo/refactor-performance to main February 27, 2024 19:13
hammerhead and others added 2 commits February 27, 2024 20:14
This is relevant content for improving SELECT performance.
This is relevant content for improving SELECT performance.
@amotl amotl force-pushed the amo/performance-downsampling branch from 6287e91 to e9a0c9a Compare February 27, 2024 19:15
@amotl amotl merged commit 61de8f5 into main Feb 27, 2024
4 checks passed
@amotl amotl deleted the amo/performance-downsampling branch February 27, 2024 19:18
@amotl amotl mentioned this pull request Feb 29, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross linking Linking to different locations of the documentation. new content New content being added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants