') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); refactor: add a `math` module by PrettyWood · Pull Request #3916 · PRQL/prql · GitHub
Skip to content

refactor: add a math module - #3916

Merged
PrettyWood merged 4 commits into
mainfrom
add-math-module
Dec 12, 2023
Merged

refactor: add a math module#3916
PrettyWood merged 4 commits into
mainfrom
add-math-module

Conversation

@PrettyWood

@PrettyWoodPrettyWood commented Dec 11, 2023•
edited
Loading

Copy link
Copy Markdown
Collaborator
Screenshot 2023-12-11 at 13 26 52

@PrettyWoodPrettyWood mentioned this pull request Dec 11, 2023
Merged
@PrettyWoodPrettyWood changed the title refactor: create a math modulerefactor: add a math moduleDec 11, 2023
@PrettyWood

PrettyWood commented Dec 11, 2023•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

@vanillajonathan Your feedback is more than welcome!

@richb-hanover

richb-hanover commented Dec 11, 2023

Copy link
Copy Markdown
Contributor

@PrettyWood COOL! How do people "include" the math... or str... modules? (Or are they built-in?)

And probably a little farther afield, how can people define new modules? I know this was planned, but (I think) disabled. Where does it stand now? Thanks again.

@vanillajonathan

vanillajonathan commented Dec 11, 2023

Copy link
Copy Markdown
Collaborator

I think this looks very promising!

I've mentioned hash functions in #1768 and #1797. There could be a hash namespace, example hash.sha1 and hash.sha3_512.

We have two string functions; upper and lower, those could be in a string namespace.

@PrettyWood

PrettyWood commented Dec 11, 2023•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

@richb-hanovermath and str modules are available automatically since they are in fact std.math and std.str.
@vanillajonathan I added in #3913 the str module that contains lower, upper and other string related functions
If this whole approach is validated, we could definitely add other modules like hash
(personally I really want to work on a module for date + time management afterwards because for BI purposes, it's almost mandatory)

max-sixty
max-sixty reviewed Dec 11, 2023
View reviewed changes
Comment threadCHANGELOG.md Outdated
- Most standard mathematical functions are now supported: `abs`, `floor`,
`ceil`, `pi`, `exp`, `ln`, `log10`, `log`, `sqrt`, `degrees`, `radians`,
`cos`, `acos`, `sin`, `asin`, `tan`, `atan` and `pow` (@PrettyWood, #3909)
- Those mathematical functions are in the `math` module (@PrettyWood, #3916)

@max-sixtymax-sixtyDec 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you have the patience — let's integrate this with the item above, since they can be handled as one change. Fine to list more than one PR under one change.

@PrettyWoodPrettyWoodDec 11, 2023•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Done in ccc424d

@max-sixty

max-sixty commented Dec 11, 2023

Copy link
Copy Markdown
Member

I'm kinda surprised this was so easy to implement! A testament to the module work that @aljazerzen has done!

I'm definitely +1. Let's wait for @aljazerzen to have a look and then merge!

aljazerzen
aljazerzen approved these changes Dec 12, 2023
View reviewed changes

@aljazerzenaljazerzen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. It would be nice to have an test-dbs integration test, so make sure the results are what we expect them to be on all dbms.

Comment threadprqlc/prql-compiler/src/sql/operators.rs
Comment on lines +126 to +131
let operator_ident = pl::Ident::from_path(
operator_name
.split('.')
.map(String::from)
.collect::<Vec<_>>(),
);

@aljazerzenaljazerzenDec 12, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

std.sql.prql was not meant to have submodules for things other than dialect handling, but it is clearer this way.

@PrettyWood

PrettyWood commented Dec 12, 2023

Copy link
Copy Markdown
CollaboratorAuthor

It would be nice to have an test-dbs integration test, so make sure the results are what we expect them to be on all dbms.

@aljazerzen Totally agree ! This is why I introduced rstest in the str (or text or string) submodule.
My plan is to use that (and maybe some macro on top of it) to test the outputs for all the supported dialects

@aljazerzen

aljazerzen commented Dec 12, 2023

Copy link
Copy Markdown
Member

test-dbs does more than that: it executes the queries on a sample chinook database.

This does not run by default, because it takes a while to compile duckdb, but you can run it with:

cargo test --package=prql-compiler --test=integration --features=test-dbs

@PrettyWood

PrettyWood commented Dec 12, 2023

Copy link
Copy Markdown
CollaboratorAuthor

@aljazerzen Ah ok right! I'll add some in another PR. Thanks

@PrettyWood
PrettyWood merged commit 4ebf1fb into mainDec 12, 2023
@PrettyWood
PrettyWood deleted the add-math-module branch December 12, 2023 10:01
@max-sixty

max-sixty commented Dec 12, 2023

Copy link
Copy Markdown
Member

test-dbs does more than that: it executes the queries on a sample chinook database.

Yes the advantage of these is that it'll test a) on a real database, b) in CI (or locally using Docker) it'll test against every supported database. So things like quotes / CONCAT we can let the tests check.

(some details under "integration tests" here: https://prql-lang.org/book/project/contributing/development.html

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@max-sixtymax-sixtymax-sixty left review comments
@aljazerzenaljazerzenaljazerzen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@PrettyWood@richb-hanover@vanillajonathan@max-sixty@aljazerzen