Skip to content

Commit 67550f1

Browse files
authored
Refactor range/gen_series signature away from user defined (#18317)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Closes #15881 - See my notes below ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> Trying to move away from user defined signatures where possible; mainly to ensure consistency of error checking/messages. The original issue is because the function has to do this checking itself leading to inconsistency of error used (ideally shouldn't be internal). By uplifting away from a user defined signature we can make use of existing code meant to handle this checking and error messages for us. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> Defined range/generate_series signature via coercible API instead of being user defined. Some accompanying changes are needed in the signature code to make this possible. ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> Added SLT tests and fixed any existing ones. ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> No (error messages do change though) <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
1 parent d2b93f5 commit 67550f1

File tree

5 files changed

+210
-97
lines changed

5 files changed

+210
-97
lines changed

datafusion/common/src/types/builtin.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,17 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
use arrow::datatypes::IntervalUnit::*;
19+
1820
use crate::types::{LogicalTypeRef, NativeType};
1921
use std::sync::{Arc, LazyLock};
2022

23+
/// Create a singleton and accompanying static variable for a [`LogicalTypeRef`]
24+
/// of a [`NativeType`].
25+
/// * `name`: name of the static variable, must be unique.
26+
/// * `getter`: name of the public function that will return the singleton instance
27+
/// of the static variable.
28+
/// * `ty`: the [`NativeType`].
2129
macro_rules! singleton {
2230
($name:ident, $getter:ident, $ty:ident) => {
2331
static $name: LazyLock<LogicalTypeRef> =
@@ -31,6 +39,26 @@ macro_rules! singleton {
3139
};
3240
}
3341

42+
/// Similar to [`singleton`], but for native types that have variants, such as
43+
/// `NativeType::Interval(MonthDayNano)`.
44+
/// * `name`: name of the static variable, must be unique.
45+
/// * `getter`: name of the public function that will return the singleton instance
46+
/// of the static variable.
47+
/// * `ty`: the [`NativeType`].
48+
/// * `variant`: specific variant of the `ty`.
49+
macro_rules! singleton_variant {
50+
($name:ident, $getter:ident, $ty:ident, $variant:ident) => {
51+
static $name: LazyLock<LogicalTypeRef> =
52+
LazyLock::new(|| Arc::new(NativeType::$ty($variant)));
53+
54+
#[doc = "Getter for singleton instance of a logical type representing"]
55+
#[doc = concat!("[`NativeType::", stringify!($ty), "`] of unit [`", stringify!($variant),"`].`")]
56+
pub fn $getter() -> LogicalTypeRef {
57+
Arc::clone(&$name)
58+
}
59+
};
60+
}
61+
3462
singleton!(LOGICAL_NULL, logical_null, Null);
3563
singleton!(LOGICAL_BOOLEAN, logical_boolean, Boolean);
3664
singleton!(LOGICAL_INT8, logical_int8, Int8);
@@ -47,3 +75,10 @@ singleton!(LOGICAL_FLOAT64, logical_float64, Float64);
4775
singleton!(LOGICAL_DATE, logical_date, Date);
4876
singleton!(LOGICAL_BINARY, logical_binary, Binary);
4977
singleton!(LOGICAL_STRING, logical_string, String);
78+
79+
singleton_variant!(
80+
LOGICAL_INTERVAL_MDN,
81+
logical_interval_mdn,
82+
Interval,
83+
MonthDayNano
84+
);

datafusion/common/src/types/native.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,4 +486,9 @@ impl NativeType {
486486
pub fn is_binary(&self) -> bool {
487487
matches!(self, NativeType::Binary | NativeType::FixedSizeBinary(_))
488488
}
489+
490+
#[inline]
491+
pub fn is_null(&self) -> bool {
492+
matches!(self, NativeType::Null)
493+
}
489494
}

datafusion/expr-common/src/signature.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,10 +382,7 @@ impl TypeSignatureClass {
382382
}
383383

384384
/// Does the specified `NativeType` match this type signature class?
385-
pub fn matches_native_type(
386-
self: &TypeSignatureClass,
387-
logical_type: &NativeType,
388-
) -> bool {
385+
pub fn matches_native_type(&self, logical_type: &NativeType) -> bool {
389386
if logical_type == &NativeType::Null {
390387
return true;
391388
}
@@ -431,6 +428,7 @@ impl TypeSignatureClass {
431428
TypeSignatureClass::Binary if native_type.is_binary() => {
432429
Ok(origin_type.to_owned())
433430
}
431+
_ if native_type.is_null() => Ok(origin_type.to_owned()),
434432
_ => internal_err!("May miss the matching logic in `matches_native_type`"),
435433
}
436434
}

0 commit comments

Comments
 (0)