Skip to content

Commit 62a35d1

Browse files
andrew-colemanandrew-coleman
committed
chore: tidy up code
Signed-off-by: Andrew Coleman <andrew_coleman@uk.ibm.com>
1 parent 1543ee7 commit 62a35d1

File tree

6 files changed

+73
-62
lines changed

6 files changed

+73
-62
lines changed

‎py/tests/data.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -867,8 +867,12 @@
867867
}]
868868
}
869869
},
870-
"offset": "0",
871-
"count": "10"
870+
"offsetMode": {
871+
"offset": "0"
872+
},
873+
"countMode": {
874+
"count": "10"
875+
}
872876
}
873877
},
874878
"names": ["L_ORDERKEY", "REVENUE", "O_ORDERDATE", "O_SHIPPRIORITY"]

‎rs/src/parse/expressions/literals.rs‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,24 @@ fn parse_user_defined(
11191119
nullable: bool,
11201120
variations: Option<extension::simple::type_variation::ResolutionResult>,
11211121
) -> diagnostic::Result<Literal> {
1122-
let extension_type = proto_required_field!(x, y, type_anchor_type, parse_type_anchor_type).1;
1122+
// Handle the new oneof structure for type_anchor_type
1123+
let extension_type = match &x.type_anchor_type {
1124+
Some(substrait::expression::literal::user_defined::TypeAnchorType::TypeReference(
1125+
type_ref,
1126+
)) => extensions::simple::parse_type_reference(type_ref, y).ok(),
1127+
Some(substrait::expression::literal::user_defined::TypeAnchorType::TypeAliasReference(
1128+
_,
1129+
)) => {
1130+
diagnostic!(
1131+
y,
1132+
Warning,
1133+
NotYetImplemented,
1134+
"type_alias_reference not yet implemented"
1135+
);
1136+
None
1137+
}
1138+
None => None,
1139+
};
11231140
proto_required_field!(x, y, val, parse_value);
11241141
let class = if let Some(extension_type) = extension_type {
11251142
data::Class::UserDefined(extension_type)
@@ -1133,20 +1150,6 @@ fn parse_user_defined(
11331150
})
11341151
}
11351152

1136-
fn parse_type_anchor_type(
1137-
x: &substrait::expression::literal::user_defined::TypeAnchorType,
1138-
y: &mut context::Context,
1139-
) -> diagnostic::Result<extension::simple::type_class::Reference> {
1140-
Ok(match x {
1141-
substrait::expression::literal::user_defined::TypeAnchorType::TypeReference(x) => {
1142-
extensions::simple::parse_type_reference(x, y)?
1143-
}
1144-
substrait::expression::literal::user_defined::TypeAnchorType::TypeAliasReference(x) => {
1145-
extensions::simple::parse_type_reference(x, y)? // TODO fix this
1146-
}
1147-
})
1148-
}
1149-
11501153
/// Parse a literal value. Returns the parsed literal.
11511154
fn parse_literal_type(
11521155
x: &substrait::expression::literal::LiteralType,
@@ -1199,7 +1202,10 @@ fn parse_literal_type(
11991202
NotYetImplemented,
12001203
"PrecisionTimestampTz literals are not yet implemented"
12011204
)),
1202-
&LiteralType::PrecisionTime(_) => todo!(),
1205+
LiteralType::PrecisionTime(_) => Err(cause!(
1206+
NotYetImplemented,
1207+
"PrecisionTime literals are not yet implemented"
1208+
)),
12031209
}
12041210
}
12051211

‎rs/src/parse/expressions/mod.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,12 @@ fn parse_expression_type(
205205
// Continue with the rest of the plan; this is not a fatal error.
206206
return Ok(Expression::Unresolved.into());
207207
}
208-
substrait::expression::RexType::DynamicParameter(_) => todo!(),
208+
substrait::expression::RexType::DynamicParameter(_) => {
209+
return Err(cause!(
210+
NotYetImplemented,
211+
"DynamicParameter expressions are not yet implemented"
212+
));
213+
}
209214
})
210215
}
211216

‎rs/src/parse/relations/fetch.rs‎

Lines changed: 32 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,69 +9,61 @@
99
1010
use crate::input::proto::substrait;
1111
use crate::output::diagnostic;
12-
use crate::output::type_system::data;
1312
use crate::parse::context;
14-
use crate::parse::expressions;
1513
use crate::util;
1614

1715
/// Parse fetch relation.
1816
pub fn parse_fetch_rel(
1917
x: &substrait::FetchRel,
2018
y: &mut context::Context,
2119
) -> diagnostic::Result<()> {
22-
use substrait::fetch_rel::CountMode;
23-
use substrait::fetch_rel::OffsetMode;
24-
2520
// Parse input.
2621
let in_type = handle_rel_input!(x, y);
2722

2823
// Filters pass through their input schema unchanged.
2924
y.set_schema(in_type);
3025

31-
// Parse offset and count.
32-
// proto_primitive_field!(x, y, offset, |x, y| {
33-
// if *x < 0 {
34-
// diagnostic!(y, Error, IllegalValue, "offsets cannot be negative");
35-
// }
36-
// Ok(())
37-
// });
38-
// proto_primitive_field!(x, y, count, |x, y| {
39-
// if *x < 0 {
40-
// diagnostic!(y, Error, IllegalValue, "count cannot be negative");
41-
// }
42-
// Ok(())
43-
// });
44-
45-
proto_field!(x, y, offset_mode); // TODO add the check for negative values
26+
// Parse offset and count from the new oneof fields.
27+
// Extract offset value (default to 0 if not set)
4628
let offset = match &x.offset_mode {
47-
Some(OffsetMode::Offset(n)) => *n,
48-
Some(OffsetMode::OffsetExpr(expr)) => {
49-
let ex: expressions::Expression = expressions::parse_expression(expr, y)?;
50-
match ex {
51-
expressions::Expression::Literal(literal) => match literal.data_type().class() {
52-
data::Class::Simple(data::class::Simple::I64) => todo!(),
53-
_ => todo!(),
54-
},
55-
_ => todo!(),
29+
Some(substrait::fetch_rel::OffsetMode::Offset(val)) => {
30+
if *val < 0 {
31+
diagnostic!(y, Error, IllegalValue, "offsets cannot be negative");
5632
}
33+
*val
34+
}
35+
Some(substrait::fetch_rel::OffsetMode::OffsetExpr(_)) => {
36+
// For now, we can't evaluate expressions, so we'll just note it
37+
diagnostic!(
38+
y,
39+
Warning,
40+
NotYetImplemented,
41+
"offset_expr evaluation not yet implemented"
42+
);
43+
0
5744
}
5845
None => 0,
5946
};
6047

61-
proto_field!(x, y, count_mode); // TODO add the check for negative values
48+
// Extract count value (default to -1 for ALL if not set)
6249
let count = match &x.count_mode {
63-
Some(CountMode::Count(n)) => *n,
64-
Some(CountMode::CountExpr(expr)) => {
65-
let ex: expressions::Expression = expressions::parse_expression(expr, y)?;
66-
match ex {
67-
expressions::Expression::Literal(literal) => match literal.data_type().class() {
68-
data::Class::Simple(data::class::Simple::I64) => todo!(),
69-
_ => todo!(),
70-
},
71-
_ => todo!(),
50+
Some(substrait::fetch_rel::CountMode::Count(val)) => {
51+
if *val < 0 {
52+
diagnostic!(y, Error, IllegalValue, "count cannot be negative");
7253
}
54+
*val
7355
}
74-
None => 0,
56+
Some(substrait::fetch_rel::CountMode::CountExpr(_)) => {
57+
// For now, we can't evaluate expressions, so we'll just note it
58+
diagnostic!(
59+
y,
60+
Warning,
61+
NotYetImplemented,
62+
"count_expr evaluation not yet implemented"
63+
);
64+
-1
65+
}
66+
None => -1,
7567
};
7668

7769
// Describe the relation.

‎rs/src/parse/relations/read.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,10 @@ fn parse_read_type(
322322
substrait::read_rel::ReadType::LocalFiles(x) => parse_local_files(x, y),
323323
substrait::read_rel::ReadType::NamedTable(x) => parse_named_table(x, y),
324324
substrait::read_rel::ReadType::ExtensionTable(x) => parse_extension_table(x, y),
325-
substrait::read_rel::ReadType::IcebergTable(_) => todo!(),
325+
substrait::read_rel::ReadType::IcebergTable(_) => Err(cause!(
326+
NotYetImplemented,
327+
"IcebergTable read type is not yet implemented"
328+
)),
326329
}
327330
}
328331

‎rs/src/parse/types.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,9 @@ pub fn parse_type_kind(
595595
substrait::r#type::Kind::UserDefined(x) => parse_user_defined(x, y),
596596
substrait::r#type::Kind::IntervalCompound(_)
597597
| substrait::r#type::Kind::PrecisionTimestamp(_)
598-
| substrait::r#type::Kind::PrecisionTimestampTz(_) => {
598+
| substrait::r#type::Kind::PrecisionTimestampTz(_)
599+
| substrait::r#type::Kind::PrecisionTime(_)
600+
| substrait::r#type::Kind::Alias(_) => {
599601
diagnostic!(
600602
y,
601603
Warning,
@@ -605,7 +607,6 @@ pub fn parse_type_kind(
605607
);
606608
Ok(())
607609
}
608-
&substrait::r#type::Kind::PrecisionTime(_) | &substrait::r#type::Kind::Alias(_) => todo!(),
609610
}
610611
}
611612

0 commit comments

Comments
 (0)