fix: data type for static schema #1235
Conversation
WalkthroughThis pull request introduces a new boolean parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Event as Event
participant Validator as Validator (Validation Logic)
Caller->>Event: to_data(..., static_schema_flag)
Event->>Validator: fields_mismatch(..., static_schema_flag)
Validator->>Validator: valid_type(..., static_schema_flag)
Validator-->>Event: Validation result
Event-->>Caller: Data conversion result
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/event/format/mod.rs (1)
105-105: Add a doc comment for the new parameter.This parameter significantly changes how data type validation is performed. Adding a brief explanation (e.g., what “static schema” implies) helps future maintainers and users understand its purpose.
/// Converts data into the appropriate format... fn to_data( self, schema: &HashMap<String, Arc<Field>>, time_partition: Option<&String>, schema_version: SchemaVersion, + /// If true, interpret certain string values as integers/floats. static_schema_flag: bool, ) -> Result<(Self::Data, EventSchema, bool), AnyError>;src/event/format/json.rs (3)
65-65: Provide brief documentation for the newstatic_schema_flag.A small inline comment or doc attribute helps clarify the reasoning for including this parameter and how it affects parsing logic.
fn to_data( self, schema: &HashMap<String, Arc<Field>>, time_partition: Option<&String>, schema_version: SchemaVersion, + /// If true, allows parsing string values as integer/float for static schema validation. static_schema_flag: bool, ) -> Result<(Self::Data, Vec<Arc<Field>>, bool), anyhow::Error> {
277-282: Add explanation ofstatic_schema_flaginvalid_type.This function’s responsibility expands with the new parameter. Consider adding a short doc comment to help clarify the branching logic.
283-306: Consider trimming input strings before parsing.Leading/ trailing whitespace or other formatting in numeric strings could break parsing in real-world data.
- if let Value::String(s) = value { - return s.parse::<i64>().is_ok(); + if let Value::String(s) = value { + return s.trim().parse::<i64>().is_ok(); }Similarly for the float parsing block:
- return s.parse::<f64>().is_ok() || s.parse::<i64>().is_ok(); + let trimmed = s.trim(); + return trimmed.parse::<f64>().is_ok() || trimmed.parse::<i64>().is_ok();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/event/format/json.rs(5 hunks)src/event/format/mod.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: coverage
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
🔇 Additional comments (4)
src/event/format/mod.rs (1)
121-126: Invocation and parameter order look correct.The call to
to_datafollows the updated signature precisely, ensuring thestatic_schema_flagis correctly passed. No issues spotted here.src/event/format/json.rs (3)
115-115: Correct propagation ofstatic_schema_flag.Passing the flag to
fields_mismatchaccurately extends the static schema checks. Implementation looks good.
257-262: Function signature looks consistent with new parameter.The updated
fields_mismatchfunction aligns with the revised type-validation flow. No major concerns.
270-270: Properly forwarding the flag tovalid_type.This ensures the integer/float parsing logic can be triggered. Code is correct.
if string parsable to int, consider it valid if string parsable to float, consider it valid
nikhilsinhaparseable
force-pushed
the
data-type-changes-v2
branch
from
ef72675 to
82fe3ec
Compare
if string parsable to int, consider it valid
if string parsable to float, consider it valid
Summary by CodeRabbit
New Features
Date32values in the casting mechanism, allowing date representations to be processed as integers.Refactor