-
-
Notifications
You must be signed in to change notification settings - Fork 254
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
Fix date_bin
fn someways (like a cat?)
#1414
Fix date_bin
fn someways (like a cat?)
#1414
Conversation
This neither is used internally nor is it publicly available, due to invalid cfg attributes on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take over this PR and do something different per the comment over there on line 124
) -> Timestamp { | ||
unsafe { | ||
direct_function_call( | ||
pg_sys::date_bin, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha. This should have been timestamp_bin
and/or timestamptz_bin
per Discord chat, we can instead add a bin(stride, origin)
function to those two types instead.
/// SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-01-01 00:02:30'); | ||
/// Result: 2020-02-11 15:32:30 | ||
/// ``` | ||
#[cfg(any(features = "pg14", features = "pg15"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/features/feature/g
😵💫 🌴
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that cargo clippy
catches these - another reason to add clippy to CI ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please examine our linting checks as they currently stand:
pgrx/.github/workflows/tests.yml
Lines 353 to 376 in 395c975
- name: Run rustfmt | |
run: cargo fmt --all -- --check | |
- name: Run license check | |
run: cargo install cargo-deny --force && ./ci/license-check.sh | |
- name: Install cargo-pgrx | |
run: cargo install --path cargo-pgrx/ --debug --force | |
- name: Run 'cargo pgrx init' for ${{ matrix.version }} | |
run: cargo pgrx init --pg$PG_VER download | |
- name: Clippy -Awarnings | |
run: cargo clippy -p pgrx --features pg$PG_VER -- -Awarnings | |
- name: Clippy -Dwarnings sql-entity-graph | |
run: cargo clippy -p pgrx-sql-entity-graph -- -Dwarnings | |
- name: Check doc-links | |
run: | | |
cargo rustdoc -p pgrx --features pg$PG_VER -- \ | |
--document-private-items \ | |
-Drustdoc::broken-intra-doc-links \ | |
-Drustdoc::invalid-html-tags |
date_bin
fndate_bin
fn someways (like a cat?)
Nightly adds a lint that warns on this, and it's very annoying for building pgrx's code with nightly. Sorry, I need to go ahead and fix this. |
This neither is used internally nor is it publicly available, due to invalid cfg attributes on it. Fixing the cfg on it breaks compilation, so the code inside it isn't valid either.