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.

Sign up for GitHub

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

Jump to bottom

Change conversion for difftime to INTERVAL, not TIME #151

Merged
merged 3 commits into from
May 1, 2024
Merged

Change conversion for difftime to INTERVAL, not TIME #151

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9133b4
Change conversion for difftime to INTERVAL, not TIME
Apr 30, 2024
1b54a6f
Round
krlmlr May 1, 2024
8f96698
Fix dbQuoteLiteral()
krlmlr May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 30 additions & 30 deletions src/include/typesr.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ enum class RTypeId {
DATE,
DATE_INTEGER,
TIMESTAMP,
TIME_SECONDS,
TIME_MINUTES,
TIME_HOURS,
TIME_DAYS,
TIME_WEEKS,
TIME_SECONDS_INTEGER,
TIME_MINUTES_INTEGER,
TIME_HOURS_INTEGER,
TIME_DAYS_INTEGER,
TIME_WEEKS_INTEGER,
INTERVAL_SECONDS,
INTERVAL_MINUTES,
INTERVAL_HOURS,
INTERVAL_DAYS,
INTERVAL_WEEKS,
INTERVAL_SECONDS_INTEGER,
INTERVAL_MINUTES_INTEGER,
INTERVAL_HOURS_INTEGER,
INTERVAL_DAYS_INTEGER,
INTERVAL_WEEKS_INTEGER,
INTEGER64,
LIST_OF_NULLS,
BLOB,
Expand Down Expand Up @@ -80,16 +80,16 @@ struct RType {
static constexpr const RTypeId DATE = RTypeId::DATE;
static constexpr const RTypeId DATE_INTEGER = RTypeId::DATE_INTEGER;
static constexpr const RTypeId TIMESTAMP = RTypeId::TIMESTAMP;
static constexpr const RTypeId TIME_SECONDS = RTypeId::TIME_SECONDS;
static constexpr const RTypeId TIME_MINUTES = RTypeId::TIME_MINUTES;
static constexpr const RTypeId TIME_HOURS = RTypeId::TIME_HOURS;
static constexpr const RTypeId TIME_DAYS = RTypeId::TIME_DAYS;
static constexpr const RTypeId TIME_WEEKS = RTypeId::TIME_WEEKS;
static constexpr const RTypeId TIME_SECONDS_INTEGER = RTypeId::TIME_SECONDS_INTEGER;
static constexpr const RTypeId TIME_MINUTES_INTEGER = RTypeId::TIME_MINUTES_INTEGER;
static constexpr const RTypeId TIME_HOURS_INTEGER = RTypeId::TIME_HOURS_INTEGER;
static constexpr const RTypeId TIME_DAYS_INTEGER = RTypeId::TIME_DAYS_INTEGER;
static constexpr const RTypeId TIME_WEEKS_INTEGER = RTypeId::TIME_WEEKS_INTEGER;
static constexpr const RTypeId INTERVAL_SECONDS = RTypeId::INTERVAL_SECONDS;
static constexpr const RTypeId INTERVAL_MINUTES = RTypeId::INTERVAL_MINUTES;
static constexpr const RTypeId INTERVAL_HOURS = RTypeId::INTERVAL_HOURS;
static constexpr const RTypeId INTERVAL_DAYS = RTypeId::INTERVAL_DAYS;
static constexpr const RTypeId INTERVAL_WEEKS = RTypeId::INTERVAL_WEEKS;
static constexpr const RTypeId INTERVAL_SECONDS_INTEGER = RTypeId::INTERVAL_SECONDS_INTEGER;
static constexpr const RTypeId INTERVAL_MINUTES_INTEGER = RTypeId::INTERVAL_MINUTES_INTEGER;
static constexpr const RTypeId INTERVAL_HOURS_INTEGER = RTypeId::INTERVAL_HOURS_INTEGER;
static constexpr const RTypeId INTERVAL_DAYS_INTEGER = RTypeId::INTERVAL_DAYS_INTEGER;
static constexpr const RTypeId INTERVAL_WEEKS_INTEGER = RTypeId::INTERVAL_WEEKS_INTEGER;
static constexpr const RTypeId INTEGER64 = RTypeId::INTEGER64;
static constexpr const RTypeId LIST_OF_NULLS = RTypeId::LIST_OF_NULLS;
static constexpr const RTypeId BLOB = RTypeId::BLOB;
Expand Down Expand Up @@ -153,24 +153,24 @@ struct RTimestampType : public RDoubleType {
static timestamp_t Convert(double val);
};

struct RTimeSecondsType : public RDoubleType {
static dtime_t Convert(double val);
struct RIntervalSecondsType : public RDoubleType {
static interval_t Convert(double val);
};

struct RTimeMinutesType : public RDoubleType {
static dtime_t Convert(double val);
struct RIntervalMinutesType : public RDoubleType {
static interval_t Convert(double val);
};

struct RTimeHoursType : public RDoubleType {
static dtime_t Convert(double val);
struct RIntervalHoursType : public RDoubleType {
static interval_t Convert(double val);
};

struct RTimeDaysType : public RDoubleType {
static dtime_t Convert(double val);
struct RIntervalDaysType : public RDoubleType {
static interval_t Convert(double val);
};

struct RTimeWeeksType : public RDoubleType {
static dtime_t Convert(double val);
struct RIntervalWeeksType : public RDoubleType {
static interval_t Convert(double val);
};

struct RIntegerType {
Expand Down
61 changes: 30 additions & 31 deletions src/scan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ data_ptr_t GetColDataPtr(const RType &rtype, SEXP coldata) {
return (data_ptr_t)DATAPTR_RO(coldata);
case RType::TIMESTAMP:
return (data_ptr_t)NUMERIC_POINTER(coldata);
case RType::TIME_SECONDS:
case RType::TIME_MINUTES:
case RType::TIME_HOURS:
case RType::TIME_DAYS:
case RType::TIME_WEEKS:
case RType::INTERVAL_SECONDS:
case RType::INTERVAL_MINUTES:
case RType::INTERVAL_HOURS:
case RType::INTERVAL_DAYS:
case RType::INTERVAL_WEEKS:
return (data_ptr_t)NUMERIC_POINTER(coldata);
case RType::TIME_SECONDS_INTEGER:
case RType::TIME_MINUTES_INTEGER:
case RType::TIME_HOURS_INTEGER:
case RType::TIME_DAYS_INTEGER:
case RType::TIME_WEEKS_INTEGER:
case RType::INTERVAL_SECONDS_INTEGER:
case RType::INTERVAL_MINUTES_INTEGER:
case RType::INTERVAL_HOURS_INTEGER:
case RType::INTERVAL_DAYS_INTEGER:
case RType::INTERVAL_WEEKS_INTEGER:
return (data_ptr_t)INTEGER_POINTER(coldata);
case RType::DATE:
if (!IS_NUMERIC(coldata)) {
Expand Down Expand Up @@ -181,54 +181,54 @@ void AppendAnyColumnSegment(const RType &rtype, bool experimental, data_ptr_t co
AppendColumnSegment<double, timestamp_t, RTimestampType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_SECONDS: {
case RType::INTERVAL_SECONDS: {
auto data_ptr = (double *)coldata_ptr;
AppendColumnSegment<double, dtime_t, RTimeSecondsType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<double, interval_t, RIntervalSecondsType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_MINUTES: {
case RType::INTERVAL_MINUTES: {
auto data_ptr = (double *)coldata_ptr;
AppendColumnSegment<double, dtime_t, RTimeMinutesType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<double, interval_t, RIntervalMinutesType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_HOURS: {
case RType::INTERVAL_HOURS: {
auto data_ptr = (double *)coldata_ptr;
AppendColumnSegment<double, dtime_t, RTimeHoursType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<double, interval_t, RIntervalHoursType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_DAYS: {
case RType::INTERVAL_DAYS: {
auto data_ptr = (double *)coldata_ptr;
AppendColumnSegment<double, dtime_t, RTimeDaysType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<double, interval_t, RIntervalDaysType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_WEEKS: {
case RType::INTERVAL_WEEKS: {
auto data_ptr = (double *)coldata_ptr;
AppendColumnSegment<double, dtime_t, RTimeWeeksType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<double, interval_t, RIntervalWeeksType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_SECONDS_INTEGER: {
case RType::INTERVAL_SECONDS_INTEGER: {
auto data_ptr = (int *)coldata_ptr;
AppendColumnSegment<int, dtime_t, RTimeSecondsType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<int, interval_t, RIntervalSecondsType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_MINUTES_INTEGER: {
case RType::INTERVAL_MINUTES_INTEGER: {
auto data_ptr = (int *)coldata_ptr;
AppendColumnSegment<int, dtime_t, RTimeMinutesType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<int, interval_t, RIntervalMinutesType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_HOURS_INTEGER: {
case RType::INTERVAL_HOURS_INTEGER: {
auto data_ptr = (int *)coldata_ptr;
AppendColumnSegment<int, dtime_t, RTimeHoursType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<int, interval_t, RIntervalHoursType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_DAYS_INTEGER: {
case RType::INTERVAL_DAYS_INTEGER: {
auto data_ptr = (int *)coldata_ptr;
AppendColumnSegment<int, dtime_t, RTimeDaysType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<int, interval_t, RIntervalDaysType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::TIME_WEEKS_INTEGER: {
case RType::INTERVAL_WEEKS_INTEGER: {
auto data_ptr = (int *)coldata_ptr;
AppendColumnSegment<int, dtime_t, RTimeWeeksType>(data_ptr, sexp_offset, v, this_count);
AppendColumnSegment<int, interval_t, RIntervalWeeksType>(data_ptr, sexp_offset, v, this_count);
break;
}
case RType::DATE: {
Expand Down Expand Up @@ -405,7 +405,6 @@ static void DataFrameScanFunc(ClientContext &context, TableFunctionInput &data,
auto rtype = bind_data.rtypes[src_df_col_idx];
AppendAnyColumnSegment(rtype, bind_data.experimental, coldata_ptr, sexp_offset, v, this_count);
}

operator_data.position += this_count;
}

Expand Down
63 changes: 31 additions & 32 deletions src/types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ RType RApiTypes::DetectRType(SEXP v, bool integer64) {
}
SEXP units0 = STRING_ELT(units, 0);
if (units0 == RStrings::get().secs) {
return RType::TIME_SECONDS;
return RType::INTERVAL_SECONDS;
} else if (units0 == RStrings::get().mins) {
return RType::TIME_MINUTES;
return RType::INTERVAL_MINUTES;
} else if (units0 == RStrings::get().hours) {
return RType::TIME_HOURS;
return RType::INTERVAL_HOURS;
} else if (units0 == RStrings::get().days) {
return RType::TIME_DAYS;
return RType::INTERVAL_DAYS;
} else if (units0 == RStrings::get().weeks) {
return RType::TIME_WEEKS;
return RType::INTERVAL_WEEKS;
} else {
return RType::UNKNOWN;
}
Expand All @@ -118,15 +118,15 @@ RType RApiTypes::DetectRType(SEXP v, bool integer64) {
}
SEXP units0 = STRING_ELT(units, 0);
if (units0 == RStrings::get().secs) {
return RType::TIME_SECONDS_INTEGER;
return RType::INTERVAL_SECONDS_INTEGER;
} else if (units0 == RStrings::get().mins) {
return RType::TIME_MINUTES_INTEGER;
return RType::INTERVAL_MINUTES_INTEGER;
} else if (units0 == RStrings::get().hours) {
return RType::TIME_HOURS_INTEGER;
return RType::INTERVAL_HOURS_INTEGER;
} else if (units0 == RStrings::get().days) {
return RType::TIME_DAYS_INTEGER;
return RType::INTERVAL_DAYS_INTEGER;
} else if (units0 == RStrings::get().weeks) {
return RType::TIME_WEEKS_INTEGER;
return RType::INTERVAL_WEEKS_INTEGER;
} else {
return RType::UNKNOWN;
}
Expand Down Expand Up @@ -223,18 +223,17 @@ LogicalType RApiTypes::LogicalTypeFromRType(const RType &rtype, bool experimenta
break;
case RType::TIMESTAMP:
return LogicalType::TIMESTAMP;
case RType::TIME_SECONDS:
case RType::TIME_MINUTES:
case RType::TIME_HOURS:
case RType::TIME_DAYS:
case RType::TIME_WEEKS:
return LogicalType::TIME;
case RType::TIME_SECONDS_INTEGER:
case RType::TIME_MINUTES_INTEGER:
case RType::TIME_HOURS_INTEGER:
case RType::TIME_DAYS_INTEGER:
case RType::TIME_WEEKS_INTEGER:
return LogicalType::TIME;
case RType::INTERVAL_SECONDS:
case RType::INTERVAL_MINUTES:
case RType::INTERVAL_HOURS:
case RType::INTERVAL_DAYS:
case RType::INTERVAL_WEEKS:
case RType::INTERVAL_SECONDS_INTEGER:
case RType::INTERVAL_MINUTES_INTEGER:
case RType::INTERVAL_HOURS_INTEGER:
case RType::INTERVAL_DAYS_INTEGER:
case RType::INTERVAL_WEEKS_INTEGER:
return LogicalType::INTERVAL;
case RType::DATE:
return LogicalType::DATE;
case RType::DATE_INTEGER:
Expand Down Expand Up @@ -332,24 +331,24 @@ timestamp_t RTimestampType::Convert(double val) {
return Timestamp::FromEpochMicroSeconds(round(val * Interval::MICROS_PER_SEC));
}

dtime_t RTimeSecondsType::Convert(double val) {
return dtime_t(int64_t(val * Interval::MICROS_PER_SEC));
interval_t RIntervalSecondsType::Convert(double val) {
return Interval::FromMicro(int64_t(val * Interval::MICROS_PER_SEC));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is consistent with the old implementation, but when converting datetimes I think we're rounding. Should we change this as part of the change here and further below?

Suggested change
return Interval::FromMicro(int64_t(val * Interval::MICROS_PER_SEC));
return Interval::FromMicro(int64_t(round(val * Interval::MICROS_PER_SEC)));

}

dtime_t RTimeMinutesType::Convert(double val) {
return dtime_t(int64_t(val * Interval::MICROS_PER_MINUTE));
interval_t RIntervalMinutesType::Convert(double val) {
return Interval::FromMicro(int64_t(val * Interval::MICROS_PER_MINUTE));
}

dtime_t RTimeHoursType::Convert(double val) {
return dtime_t(int64_t(val * Interval::MICROS_PER_HOUR));
interval_t RIntervalHoursType::Convert(double val) {
return Interval::FromMicro(int64_t(val * Interval::MICROS_PER_HOUR));
}

dtime_t RTimeDaysType::Convert(double val) {
return dtime_t(int64_t(val * Interval::MICROS_PER_DAY));
interval_t RIntervalDaysType::Convert(double val) {
return Interval::FromMicro(int64_t(val * Interval::MICROS_PER_DAY));
}

dtime_t RTimeWeeksType::Convert(double val) {
return dtime_t(int64_t(val * (Interval::MICROS_PER_DAY * Interval::DAYS_PER_WEEK)));
interval_t RIntervalWeeksType::Convert(double val) {
return Interval::FromMicro(int64_t(val * (Interval::MICROS_PER_DAY * Interval::DAYS_PER_WEEK)));
}

bool RIntegerType::IsNull(int val) {
Expand Down
44 changes: 22 additions & 22 deletions src/utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,47 +160,47 @@ Value RApiTypes::SexpToValue(SEXP valsexp, R_len_t idx, bool typed_logical_null)
auto d_val = INTEGER_POINTER(valsexp)[idx];
return RIntegerType::IsNull(d_val) ? Value(LogicalType::DATE) : Value::DATE(RDateType::Convert(d_val));
}
case RType::TIME_SECONDS: {
case RType::INTERVAL_SECONDS: {
auto ts_val = NUMERIC_POINTER(valsexp)[idx];
return RTimeSecondsType::IsNull(ts_val) ? Value(LogicalType::TIME)
: Value::TIME(RTimeSecondsType::Convert(ts_val));
return RIntervalSecondsType::IsNull(ts_val) ? Value(LogicalType::INTERVAL)
: Value::INTERVAL(RIntervalSecondsType::Convert(ts_val));
}
case RType::TIME_MINUTES: {
case RType::INTERVAL_MINUTES: {
auto ts_val = NUMERIC_POINTER(valsexp)[idx];
return RTimeMinutesType::IsNull(ts_val) ? Value(LogicalType::TIME)
: Value::TIME(RTimeMinutesType::Convert(ts_val));
return RIntervalMinutesType::IsNull(ts_val) ? Value(LogicalType::INTERVAL)
: Value::INTERVAL(RIntervalMinutesType::Convert(ts_val));
}
case RType::TIME_HOURS: {
case RType::INTERVAL_HOURS: {
auto ts_val = NUMERIC_POINTER(valsexp)[idx];
return RTimeHoursType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeHoursType::Convert(ts_val));
return RIntervalHoursType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalHoursType::Convert(ts_val));
}
case RType::TIME_DAYS: {
case RType::INTERVAL_DAYS: {
auto ts_val = NUMERIC_POINTER(valsexp)[idx];
return RTimeDaysType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeDaysType::Convert(ts_val));
return RIntervalDaysType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalDaysType::Convert(ts_val));
}
case RType::TIME_WEEKS: {
case RType::INTERVAL_WEEKS: {
auto ts_val = NUMERIC_POINTER(valsexp)[idx];
return RTimeWeeksType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeWeeksType::Convert(ts_val));
return RIntervalWeeksType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalWeeksType::Convert(ts_val));
}
case RType::TIME_SECONDS_INTEGER: {
case RType::INTERVAL_SECONDS_INTEGER: {
auto ts_val = INTEGER_POINTER(valsexp)[idx];
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeSecondsType::Convert(ts_val));
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalSecondsType::Convert(ts_val));
}
case RType::TIME_MINUTES_INTEGER: {
case RType::INTERVAL_MINUTES_INTEGER: {
auto ts_val = INTEGER_POINTER(valsexp)[idx];
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeMinutesType::Convert(ts_val));
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalMinutesType::Convert(ts_val));
}
case RType::TIME_HOURS_INTEGER: {
case RType::INTERVAL_HOURS_INTEGER: {
auto ts_val = INTEGER_POINTER(valsexp)[idx];
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeHoursType::Convert(ts_val));
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalHoursType::Convert(ts_val));
}
case RType::TIME_DAYS_INTEGER: {
case RType::INTERVAL_DAYS_INTEGER: {
auto ts_val = INTEGER_POINTER(valsexp)[idx];
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeDaysType::Convert(ts_val));
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalDaysType::Convert(ts_val));
}
case RType::TIME_WEEKS_INTEGER: {
case RType::INTERVAL_WEEKS_INTEGER: {
auto ts_val = INTEGER_POINTER(valsexp)[idx];
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::TIME) : Value::TIME(RTimeWeeksType::Convert(ts_val));
return RIntegerType::IsNull(ts_val) ? Value(LogicalType::INTERVAL) : Value::INTERVAL(RIntervalWeeksType::Convert(ts_val));
}
case RType::LIST_OF_NULLS:
// Performance shortcut: this corresponds to the RType::BLOB case,
Expand Down
Loading