Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

chore: Don't wrap `TimeRange` in an `Option` 

It is unnecessary, because both the lower and upper are already
optional values.
pull/24376/head
Stuart Carnie 2023-06-06 15:07:47 +10:00
parent 9e2550c933
commit 8c71f30145
No known key found for this signature in database
GPG Key ID: 848D9C9718D78B4F
5 changed files with 58 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
influxdb_influxql_parser/src/time_range.rs
View File

@ -178,9 +178,9 @@ pub type ExprResult = Result<Expr, ExprError>;
pub fn split_cond(
ctx: &ReduceContext,
cond: &ConditionalExpression,
) -> Result<(Option<ConditionalExpression>, Option<TimeRange>), ExprError> {
) -> Result<(Option<ConditionalExpression>, TimeRange), ExprError> {
if !has_time_range(cond) {
return Ok((Some(cond.clone()), None));
return Ok((Some(cond.clone()), TimeRange::default()));
}
let mut time_range = TimeRange::default();
@ -328,7 +328,7 @@ pub fn split_cond(
.pop()
.ok_or_else(|| error::map::internal("expected an element on stack"))?;
Ok((cond, Some(time_range)))
Ok((cond, time_range))
}
/// Search `cond` for expressions involving the `time` column.
@ -356,6 +356,11 @@ pub struct TimeRange {
}
impl TimeRange {
/// Returns `true` if the `lower` and `upper` bounds are `None`.
pub fn is_unbounded(self) -> bool {
self.lower.is_none() && self.upper.is_none()
}
fn intersect(&mut self, other: TimeRange) {
if let Some(other) = other.lower {
match self.lower {
@ -719,9 +724,7 @@ mod test {
#[test]
fn test_split_cond() {
fn split_exprs(
s: &str,
) -> Result<(Option<ConditionalExpression>, Option<TimeRange>), ExprError> {
fn split_exprs(s: &str) -> Result<(Option<ConditionalExpression>, TimeRange), ExprError> {
// 2023-01-01T00:00:00Z == 1672531200000000000
let ctx = ReduceContext {
now: Some(Timestamp::from_utc(
@ -791,36 +794,36 @@ mod test {
let (cond, tr) = split_exprs("time >= now() - 1s").unwrap();
assert!(cond.is_none());
assert_eq!(tr.unwrap(), range!(lower = 1672531199000000000));
assert_eq!(tr, range!(lower = 1672531199000000000));
// reduces the lower bound to a single expression
let (cond, tr) = split_exprs("time >= now() - 1s AND time >= now() - 500ms").unwrap();
assert!(cond.is_none());
assert_eq!(tr.unwrap(), range!(lower = 1672531199500000000));
assert_eq!(tr, range!(lower = 1672531199500000000));
let (cond, tr) = split_exprs("time <= now() - 1s").unwrap();
assert!(cond.is_none());
assert_eq!(tr.unwrap(), range!(upper = 1672531199000000000));
assert_eq!(tr, range!(upper = 1672531199000000000));
// reduces the upper bound to a single expression
let (cond, tr) = split_exprs("time <= now() + 1s AND time <= now() + 500ms").unwrap();
assert!(cond.is_none());
assert_eq!(tr.unwrap(), range!(upper = 1672531200500000000));
assert_eq!(tr, range!(upper = 1672531200500000000));
let (cond, tr) = split_exprs("time >= now() - 1s AND time < now()").unwrap();
assert!(cond.is_none());
assert_eq!(
tr.unwrap(),
tr,
range!(lower=1672531199000000000, upper ex=1672531200000000000)
);
let (cond, tr) = split_exprs("time >= now() - 1s AND cpu = 'cpu0'").unwrap();
assert_eq!(cond.unwrap().to_string(), "cpu = 'cpu0'");
assert_eq!(tr.unwrap(), range!(lower = 1672531199000000000));
assert_eq!(tr, range!(lower = 1672531199000000000));
let (cond, tr) = split_exprs("time = 0").unwrap();
assert!(cond.is_none());
assert_eq!(tr.unwrap(), range!(eq = 0));
assert_eq!(tr, range!(eq = 0));
let (cond, tr) = split_exprs(
"instance = 'instance-01' OR instance = 'instance-02' AND time >= now() - 1s",
@ -830,14 +833,14 @@ mod test {
cond.unwrap().to_string(),
"instance = 'instance-01' OR instance = 'instance-02'"
);
assert_eq!(tr.unwrap(), range!(lower = 1672531199000000000));
assert_eq!(tr, range!(lower = 1672531199000000000));
let (cond, tr) =
split_exprs("time >= now() - 1s AND time < now() AND cpu = 'cpu0' OR cpu = 'cpu1'")
.unwrap();
assert_eq!(cond.unwrap().to_string(), "cpu = 'cpu0' OR cpu = 'cpu1'");
assert_eq!(
tr.unwrap(),
tr,
range!(lower=1672531199000000000, upper ex=1672531200000000000)
);
@ -849,7 +852,7 @@ mod test {
.unwrap();
assert_eq!(cond.unwrap().to_string(), "cpu = 'cpu0' OR cpu = 'cpu1'");
assert_eq!(
tr.unwrap(),
tr,
range!(lower=1672531199000000000, upper ex=1672531200000000000)
);
@ -857,19 +860,16 @@ mod test {
assert_eq!(cond.unwrap().to_string(), "cpu = 'cpu0'");
// Models InfluxQL behaviour, which will result in no results being returned because
// upper < lower
assert_eq!(tr.unwrap(), range!(lower = 10, upper = 0));
assert_eq!(tr, range!(lower = 10, upper = 0));
// no time
let (cond, tr) = split_exprs("f64 >= 19.5 OR f64 =~ /foo/").unwrap();
assert_eq!(cond.unwrap().to_string(), "f64 >= 19.5 OR f64 =~ /foo/");
assert!(tr.is_none());
assert!(tr.is_unbounded());
let (cond, tr) = split_exprs("time > now() OR time = 1000").unwrap();
assert!(cond.is_none());
assert_eq!(
tr.unwrap(),
range!(lower ex = 1672531200000000000, upper = 1000)
);
assert_eq!(tr, range!(lower ex = 1672531200000000000, upper = 1000));
// fallible

30
iox_query_influxql/src/plan/ir.rs
View File

@ -44,7 +44,7 @@ pub(super) struct Select {
pub(super) condition: Option<ConditionalExpression>,
/// The time range derived from the `WHERE` clause of the `SELECT` statement.
pub(super) time_range: Option<TimeRange>,
pub(super) time_range: TimeRange,
/// The GROUP BY clause of the selection.
pub(super) group_by: Option<GroupByClause>,
@ -121,22 +121,20 @@ impl From<Select> for SelectStatement {
/// Combine the `condition` and `time_range` into a single `WHERE` predicate.
fn where_clause(
condition: Option<ConditionalExpression>,
time_range: Option<TimeRange>,
time_range: TimeRange,
) -> Option<WhereClause> {
let time_expr: Option<ConditionalExpression> = if let Some(t) = time_range {
Some(
match (t.lower, t.upper) {
(Some(lower), Some(upper)) if lower == upper => format!("time = {lower}"),
(Some(lower), Some(upper)) => format!("time >= {lower} AND time <= {upper}"),
(Some(lower), None) => format!("time >= {lower}"),
(None, Some(upper)) => format!("time <= {upper}"),
(None, None) => unreachable!(),
}
.parse()
.unwrap(),
)
} else {
None
let time_expr: Option<ConditionalExpression> = match (time_range.lower, time_range.upper) {
(Some(lower), Some(upper)) if lower == upper => {
Some(format!("time = {lower}").parse().unwrap())
}
(Some(lower), Some(upper)) => Some(
format!("time >= {lower} AND time <= {upper}")
.parse()
.unwrap(),
),
(Some(lower), None) => Some(format!("time >= {lower}").parse().unwrap()),
(None, Some(upper)) => Some(format!("time <= {upper}").parse().unwrap()),
(None, None) => None,
};
match (time_expr, condition) {

32
iox_query_influxql/src/plan/planner.rs
View File

@ -140,7 +140,7 @@ struct Context<'a> {
// WHERE
condition: Option<&'a ConditionalExpression>,
time_range: Option<TimeRange>,
time_range: TimeRange,
// GROUP BY information
group_by: Option<&'a GroupByClause>,
@ -169,6 +169,8 @@ impl<'a> Context<'a> {
}
}
/// Create a new context for the select statement that is
/// a subquery of the current context.
fn subquery(&self, select: &'a Select) -> Self {
Self {
table_name: self.table_name,
@ -1214,7 +1216,7 @@ impl<'a> InfluxQLToLogicalPlan<'a> {
fn plan_condition_time_range(
&self,
condition: Option<&ConditionalExpression>,
time_range: Option<TimeRange>,
time_range: TimeRange,
plan: LogicalPlan,
schemas: &Schemas,
ds_schema: &DataSourceSchema<'_>,
@ -1231,7 +1233,7 @@ impl<'a> InfluxQLToLogicalPlan<'a> {
})
.transpose()?;
let time_expr = time_range.map(time_range_to_df_expr).flatten();
let time_expr = time_range_to_df_expr(time_range);
let pb = LogicalPlanBuilder::from(plan);
match (time_expr, filter_expr) {
@ -1268,17 +1270,21 @@ impl<'a> InfluxQLToLogicalPlan<'a> {
.unwrap_or_default();
// Add time restriction to logical plan if there isn't any.
let time_range = time_range.unwrap_or_else(|| TimeRange {
lower: Some(match cutoff {
MetadataCutoff::Absolute(dt) => dt.timestamp_nanos(),
MetadataCutoff::Relative(delta) => {
start_time.timestamp_nanos() - delta.as_nanos() as i64
}
}),
upper: None,
});
let time_range = if time_range.is_unbounded() {
TimeRange {
lower: Some(match cutoff {
MetadataCutoff::Absolute(dt) => dt.timestamp_nanos(),
MetadataCutoff::Relative(delta) => {
start_time.timestamp_nanos() - delta.as_nanos() as i64
}
}),
upper: None,
}
} else {
time_range
};
self.plan_condition_time_range(cond.as_ref(), Some(time_range), plan, schemas, ds_schema)
self.plan_condition_time_range(cond.as_ref(), time_range, plan, schemas, ds_schema)
}
/// Generate a logical plan for the specified `DataSource`.

2
iox_query_influxql/src/plan/planner_time_range_expression.rs
View File

@ -45,9 +45,9 @@ pub(super) fn time_range_to_df_expr(cond: TimeRange) -> Option<DFExpr> {
// If lower > upper, then the expression lower < time < upper is always false
(Some(lower), Some(upper)) if lower > upper => Some(lit(false)),
(lower, upper) => match (lower_bound_to_df_expr(lower), upper_bound_to_df_expr(upper)) {
(None, None) => None,
(Some(e), None) | (None, Some(e)) => Some(e),
(Some(lower), Some(upper)) => Some(lower.and(upper)),
(None, None) => None,
},
}
}

4
iox_query_influxql/src/plan/rewriter.rs
View File

@ -19,7 +19,7 @@ use influxdb_influxql_parser::select::{
Dimension, FillClause, FromMeasurementClause, GroupByClause, MeasurementSelection,
SelectStatement,
};
use influxdb_influxql_parser::time_range::{split_cond, ReduceContext};
use influxdb_influxql_parser::time_range::{split_cond, ReduceContext, TimeRange};
use influxdb_influxql_parser::timestamp::Timestamp;
use itertools::Itertools;
use schema::InfluxColumnType;
@ -110,7 +110,7 @@ impl RewriteSelect {
};
split_cond(&rc, &where_clause).map_err(error::map::expr_error)?
}
None => (None, None),
None => (None, TimeRange::default()),
};
let SelectStatementInfo { projection_type } =