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

[Bitstring] Add overload for bitstring to accept BIT as the type of the first argument #14247

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

Tishj
Copy link
Contributor

@Tishj Tishj commented Oct 7, 2024

This PR fixes #14216

…st casts to a string and constructs a varchar string_t from that
@Mytherin
Copy link
Collaborator

Mytherin commented Oct 7, 2024

Thanks for the PR! LGTM - one comment

@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 8, 2024 08:25
@Tishj
Copy link
Contributor Author

Tishj commented Oct 8, 2024

I've added a using declaration to differentiate logically between a string_t and a bitstring_t so we know what the argument is supposed to represent

But in the future we should probably do this:

struct bitstring_t : public string_t { // NOLINT
};

(this is also what we do for the timestamp variants)

struct timestamp_tz_t : public timestamp_t { // NOLINT
};
struct timestamp_ns_t : public timestamp_t { // NOLINT
};
struct timestamp_ms_t : public timestamp_t { // NOLINT
};
struct timestamp_sec_t : public timestamp_t { // NOLINT
};

So this differentiation is explicit at the type system level, while still sharing the same underlying physical representation

@Tishj Tishj marked this pull request as ready for review October 8, 2024 08:38
@Tishj Tishj requested a review from Mytherin October 10, 2024 10:28
@Mytherin Mytherin merged commit d57a944 into duckdb:main Oct 10, 2024
41 of 42 checks passed
@Mytherin
Copy link
Collaborator

Thanks! Good idea to create the separate type.

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Oct 16, 2024
[Bitstring] Add overload for `bitstring` to accept BIT as the type of the first argument (duckdb/duckdb#14247)
Json bugfixes (duckdb/duckdb#14288)
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Oct 16, 2024
[Bitstring] Add overload for `bitstring` to accept BIT as the type of the first argument (duckdb/duckdb#14247)
Json bugfixes (duckdb/duckdb#14288)

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
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.

No function matches 'bitstring(BIT, INTEGER_LITERAL)'
2 participants