Skip to content

[RFC] Add ability to declare unit tests in udf.rs#4922

Draft
Karakatiza666 wants to merge 3 commits intomainfrom
udf-tests
Draft

[RFC] Add ability to declare unit tests in udf.rs#4922
Karakatiza666 wants to merge 3 commits intomainfrom
udf-tests

Conversation

@Karakatiza666
Copy link
Copy Markdown
Contributor

Compilation error in the test module, or the test failures do not prevent starting the pipeline.

You can use the following pipeline to test the feature:
program.sql

CREATE FUNCTION parse_date_custom(str VARCHAR NOT NULL)
RETURNS TIMESTAMP NOT NULL;

CREATE TABLE date_strings (
    id INTEGER NOT NULL PRIMARY KEY,
    format_name VARCHAR(100) NOT NULL,
    date_string VARCHAR(255) NOT NULL,
    description VARCHAR(255)
);
 
CREATE VIEW parsed_dates AS
SELECT 
    id,
    format_name,
    date_string,
    parse_date_custom(date_string) AS parsed_timestamp,
    description
FROM date_strings;

udf.rs

#![allow(non_snake_case)]

use feldera_sqllib::*;
use crate::*;
use chrono::{DateTime, Datelike, NaiveDate, NaiveDateTime, Timelike, TimeZone, Utc};



pub fn parse_date_custom(str: SqlString) -> Result<Timestamp, Box<dyn std::error::Error>> {
    // Handle null/empty cases
    if str.is_empty() || str == SqlString::from_ref("NULL") || str == SqlString::from_ref("null") {
        return Err("Cannot parse null or empty string".into());
    }

    // Define all the date format patterns to try
    let formats = vec![
        "%Y-%m-%d %H:%M:%S Z",
        "%Y-%m-%dT%H:%M:%S%.3fZ",
        "%Y-%m-%dT%H:%M:%SZ",
        "%Y-%m-%dT%H:%M:%S%.3f%z",
        "%Y-%m-%dT%H:%M:%S%z",
        "%Y-%m-%dT%H:%M:%S%.6f",
        "%Y-%m-%dT%H:%M:%S%.6f%z",
        "%Y-%m-%d %H:%M:%S%.6f%z",
        "%Y-%m-%dT%H:%M:%S",
        "%Y-%m-%d",
        "%Y-%m-%d %H:%M",
        "%d-%m-%Y %H:%M:%S",
        "%d-%m-%Y %H:%M",
        "%d-%m-%Y",
        "%d/%m/%Y %H:%M:%S",
        "%d/%m/%Y %H:%M",
        "%m-%d-%Y %H:%M:%S",
        "%m-%d-%Y %H:%M",
        "%m-%d-%Y",
        "%m/%d/%Y %H:%M:%S",
        "%m/%d/%Y %H:%M",
        "%m/%d/%y",
        "%m/%d/%Y",
        "%d/%m/%Y",
        "%Y%m%d",
        "%Y-%m-%d %H:%M:%S UTC",
        "%Y-%m-%d %H:%M:%S",
        "%Y-%m-%d %H:%M:%S %z",
        "%m/%d/%Y %I:%M %p",
        "%m/%d/%Y %I:%M:%S %p",
    ];

    // Try each format
    for format in formats {
        // Try parsing with timezone
        if let Ok(dt) = DateTime::parse_from_str(str.str(), format) {
            let utc_dt = dt.with_timezone(&Utc);
            let millis = utc_dt.timestamp_millis();
            return Ok(Timestamp::new(millis));
        }
    
        // Try parsing as naive datetime and assume UTC
        if let Ok(naive_dt) = NaiveDateTime::parse_from_str(&str.str(), format) {
            let utc_dt = Utc.from_utc_datetime(&naive_dt);
            let millis = utc_dt.timestamp_millis();
            return Ok(Timestamp::new(millis));
        }
        
        // Try parsing as date only
        if let Ok(naive_date) = NaiveDate::parse_from_str(&str.str(), format) {
            if let Some(naive_dt) = naive_date.and_hms_opt(0, 0, 0) {
                let utc_dt = Utc.from_utc_datetime(&naive_dt);
                let millis = utc_dt.timestamp_millis();
                return Ok(Timestamp::new(millis));
            }
        }
    }

    // Try parsing as integer timestamp (Unix epoch)
    if let Ok(int_val) = str.str().split('.').next().unwrap_or("").parse::<i64>() {
        let str_len = int_val.to_string().len();
        
        // 10-12 digits: seconds since epoch
        if str_len >= 10 && str_len < 13 {
            let millis = int_val * 1000;
            return Ok(Timestamp::new(millis));
        } 
        // Other lengths: milliseconds since epoch
        else if str_len >= 13 {
            return Ok(Timestamp::new(int_val));
        }
    }

    Err(format!("Unable to parse date string: '{}'", str).into())
}

#[cfg(test)]
mod tests {
    use super::*;

    fn assert_date_equals(result: Result<Timestamp, Box<dyn std::error::Error>>, year: i32, month: u32, day: u32, hour: u32, minute: u32, second: u32) {
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), year);
        assert_eq!(dt.month(), month);
        assert_eq!(dt.day(), day);
        assert_eq!(dt.hour(), hour);
        assert_eq!(dt.minute(), minute);
        assert_eq!(dt.second(), second);
    }

    #[test]
    fn test_null_values() {
        assert!(parse_date_custom(SqlString::from_ref("")).is_err());
        assert!(parse_date_custom(SqlString::from_ref("NULL")).is_err());
        assert!(parse_date_custom(SqlString::from_ref("null")).is_err());
    }

    #[test]
    fn test_iso8601_with_z() {
        // yyyy-MM-dd HH:mm:ss Z
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-16 14:30:45 Z")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_iso8601_with_milliseconds_z() {
        // yyyy-MM-ddTHH:mm:ss.SSSZ
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45.123Z"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_iso8601_t_format_z() {
        // yyyy-MM-ddTHH:mm:ssZ
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45Z")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_iso8601_with_milliseconds_offset() {
        // yyyy-MM-ddTHH:mm:ss.SSS+ZZZZ
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45.123+0530"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_iso8601_with_offset() {
        // yyyy-MM-ddTHH:mm:ss+ZZZZ
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45+0530"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_iso8601_with_microseconds() {
        // yyyy-MM-ddTHH:mm:ss.SSSSSS
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45.123456")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_iso8601_with_microseconds_offset() {
        // yyyy-MM-ddTHH:mm:ss.SSSSSS+ZZZZ
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45.123456+0530"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_space_format_with_microseconds_offset() {
        // yyyy-MM-dd HH:mm:ss.SSSSSS+ZZZZ
        let result = parse_date_custom(SqlString::from_ref("2024-03-15 14:30:45.123456+0530"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_iso8601_t_format() {
        // yyyy-MM-ddTHH:mm:ss
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_date_only() {
        // yyyy-MM-dd
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_date_with_hour_minute() {
        // yyyy-MM-dd HH:mm
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15 14:30")),
            2024, 3, 15, 14, 30, 0
        );
    }

    #[test]
    fn test_dd_mm_yyyy_with_time() {
        // dd-MM-yyyy HH:mm:ss
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("15-03-2024 14:30:45")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_dd_mm_yyyy_with_hour_minute() {
        // dd-MM-yyyy HH:mm
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("15-03-2024 14:30")),
            2024, 3, 15, 14, 30, 0
        );
    }

    #[test]
    fn test_dd_mm_yyyy() {
        // dd-MM-yyyy
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("15-03-2024")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_dd_slash_mm_slash_yyyy_with_time() {
        // dd/MM/yyyy HH:mm:ss
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("15/03/2024 14:30:45")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_dd_slash_mm_slash_yyyy_with_hour_minute() {
        // dd/MM/yyyy HH:mm
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("15/03/2024 14:30")),
            2024, 3, 15, 14, 30, 0
        );
    }

    #[test]
    fn test_mm_dd_yyyy_with_time() {
        // MM-dd-yyyy HH:mm:ss
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03-15-2024 14:30:45")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_mm_dd_yyyy_with_hour_minute() {
        // MM-dd-yyyy HH:mm
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03-15-2024 14:30")),
            2024, 3, 15, 14, 30, 0
        );
    }

    #[test]
    fn test_mm_dd_yyyy() {
        // MM-dd-yyyy
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03-15-2024")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_mm_slash_dd_slash_yyyy_with_time() {
        // MM/dd/yyyy HH:mm:ss
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/2024 14:30:45")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_mm_slash_dd_slash_yyyy_with_hour_minute() {
        // MM/dd/yyyy HH:mm
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/2024 14:30")),
            2024, 3, 15, 14, 30, 0
        );
    }

    #[test]
    fn test_mm_slash_dd_slash_yy() {
        // MM/dd/yy
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/24")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_mm_slash_dd_slash_yyyy() {
        // MM/dd/yyyy
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/2024")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_dd_slash_mm_slash_yyyy() {
        // dd/MM/yyyy
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("15/03/2024")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_yyyymmdd() {
        // yyyyMMdd
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("20240315")),
            2024, 3, 15, 0, 0, 0
        );
    }

    #[test]
    fn test_utc_suffix() {
        // yyyy-MM-dd HH:mm:ss UTC
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15 14:30:45 UTC")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_space_format() {
        // yyyy-MM-dd HH:mm:ss
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("2024-03-15 14:30:45")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_space_format_with_offset() {
        // yyyy-MM-dd HH:mm:ss +ZZZZ
        let result = parse_date_custom(SqlString::from_ref("2024-03-15 14:30:45 +0530"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_12_hour_format_am() {
        // MM/dd/yyyy hh:mm aa
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/2024 02:30 AM")),
            2024, 3, 15, 2, 30, 0
        );
    }

    #[test]
    fn test_12_hour_format_pm() {
        // MM/dd/yyyy hh:mm aa
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/2024 02:30 PM")),
            2024, 3, 15, 14, 30, 0
        );
    }

    #[test]
    fn test_12_hour_format_with_seconds() {
        // MM/dd/yyyy hh:mm:ss aa
        assert_date_equals(
            parse_date_custom(SqlString::from_ref("03/15/2024 02:30:45 PM")),
            2024, 3, 15, 14, 30, 45
        );
    }

    #[test]
    fn test_unix_timestamp_seconds() {
        // 10-12 digits: seconds since epoch
        let result = parse_date_custom(SqlString::from_ref("1710508245")); // March 15, 2024
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
    }

    #[test]
    fn test_unix_timestamp_milliseconds() {
        // 13+ digits: milliseconds since epoch
        let result = parse_date_custom(SqlString::from_ref("1710508245000")); // March 15, 2024
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
    }

    #[test]
    fn test_invalid_string() {
        assert!(parse_date_custom(SqlString::from_ref("not a date")).is_err());
        assert!(parse_date_custom(SqlString::from_ref("invalid/date/format")).is_err());
        assert!(parse_date_custom(SqlString::from_ref("xyz")).is_err());
    }

    #[test]
    fn test_negative_offset() {
        // Test with negative timezone offset
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45-0500"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_colon_in_offset() {
        // Test with colon in timezone offset
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45+05:30"));
        let timestamp = result.expect("Failed to parse date");
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
        assert_eq!(dt.day(), 15);
    }

    #[test]
    fn test_timestamp_milliseconds_method() {
        let result = parse_date_custom(SqlString::from_ref("2024-03-15T14:30:45Z"));
        let timestamp = result.expect("Failed to parse date");
        assert!(timestamp.milliseconds() > 0);
    }

    #[test]
    fn test_timestamp_new_and_to_datetime() {
        let millis = 1710508245000i64; // March 15, 2024
        let timestamp = Timestamp::new(millis);
        assert_eq!(timestamp.milliseconds(), millis);
        
        let dt = timestamp.to_dateTime();
        assert_eq!(dt.year(), 2024);
        assert_eq!(dt.month(), 3);
    }
}

udf.toml

chrono = "0.4"

Ad-hoc insert statement:

INSERT INTO date_strings (id, format_name, date_string, description) VALUES
    -- ISO 8601 Formats
    (1, 'ISO8601_Z', '2024-03-15 14:30:45 Z', 'ISO 8601 with space and Z'),
    (2, 'ISO8601_T_MS_Z', '2024-03-15T14:30:45.123Z', 'ISO 8601 with T, milliseconds, and Z'),
    (3, 'ISO8601_T_Z', '2024-03-15T14:30:45Z', 'ISO 8601 with T and Z'),
    (4, 'ISO8601_MS_OFFSET', '2024-03-15T14:30:45.123+0530', 'ISO 8601 with milliseconds and offset'),
    (5, 'ISO8601_OFFSET', '2024-03-15T14:30:45+0530', 'ISO 8601 with timezone offset'),
    (6, 'ISO8601_OFFSET_COLON', '2024-03-15T14:30:45+05:30', 'ISO 8601 with offset (colon)'),
    (7, 'ISO8601_OFFSET_NEG', '2024-03-15T14:30:45-0500', 'ISO 8601 with negative offset'),
    (8, 'ISO8601_MICROSEC', '2024-03-15T14:30:45.123456', 'ISO 8601 with microseconds'),
    (9, 'ISO8601_MICROSEC_OFFSET', '2024-03-15T14:30:45.123456+0530', 'ISO 8601 with microseconds and offset'),
    (10, 'ISO8601_SPACE_MICROSEC', '2024-03-15 14:30:45.123456+0530', 'ISO 8601 with space and microseconds'),
    (11, 'ISO8601_T', '2024-03-15T14:30:45', 'ISO 8601 basic with T'),
    (12, 'ISO8601_SPACE', '2024-03-15 14:30:45', 'ISO 8601 with space'),
    
    -- Date Only
    (13, 'DATE_ONLY', '2024-03-15', 'Date only (yyyy-MM-dd)'),
    (14, 'DATE_HOUR_MIN', '2024-03-15 14:30', 'Date with hour and minute'),
    
    -- European Format (dd-MM-yyyy, dd/MM/yyyy)
    (15, 'EU_DASH_TIME', '15-03-2024 14:30:45', 'European with dashes and time'),
    (16, 'EU_DASH_HM', '15-03-2024 14:30', 'European with dashes and HH:mm'),
    (17, 'EU_DASH', '15-03-2024', 'European with dashes only'),
    (18, 'EU_SLASH_TIME', '15/03/2024 14:30:45', 'European with slashes and time'),
    (19, 'EU_SLASH_HM', '15/03/2024 14:30', 'European with slashes and HH:mm'),
    (20, 'EU_SLASH', '15/03/2024', 'European with slashes only'),
    
    -- US Format (MM-dd-yyyy, MM/dd/yyyy)
    (21, 'US_DASH_TIME', '03-15-2024 14:30:45', 'US with dashes and time'),
    (22, 'US_DASH_HM', '03-15-2024 14:30', 'US with dashes and HH:mm'),
    (23, 'US_DASH', '03-15-2024', 'US with dashes only'),
    (24, 'US_SLASH_TIME', '03/15/2024 14:30:45', 'US with slashes and time'),
    (25, 'US_SLASH_HM', '03/15/2024 14:30', 'US with slashes and HH:mm'),
    (26, 'US_SLASH', '03/15/2024', 'US with slashes only'),
    (27, 'US_SLASH_SHORT', '03/15/24', 'US with short year'),
    
    -- 12-Hour Format
    (28, '12HR_AM', '03/15/2024 02:30 AM', '12-hour AM'),
    (29, '12HR_PM', '03/15/2024 02:30 PM', '12-hour PM'),
    (30, '12HR_SEC_PM', '03/15/2024 02:30:45 PM', '12-hour with seconds PM'),
    
    -- Special Formats
    (31, 'COMPACT', '20240315', 'Compact (yyyyMMdd)'),
    (32, 'UTC_SUFFIX', '2024-03-15 14:30:45 UTC', 'With UTC suffix'),
    
    -- Unix Timestamps
    (33, 'UNIX_SEC', '1710508245', 'Unix timestamp (seconds)'),
    (34, 'UNIX_MS', '1710508245000', 'Unix timestamp (milliseconds)'),
    
    -- Special Dates
    (35, 'CHRISTMAS_2024', '2024-12-25T00:00:00Z', 'Christmas 2024'),
    (36, 'NEW_YEAR_2025', '01/01/2025 00:00:00', 'New Year 2025'),
    (37, 'LEAP_DAY_2024', '29/02/2024 12:00:00', 'Leap day 2024'),
    (38, 'MILLENNIUM', '01/01/2000 00:00:00', 'Y2K');

@Karakatiza666 Karakatiza666 force-pushed the udf-tests branch 2 times, most recently from 8993061 to 9ba6b57 Compare October 16, 2025 13:24
Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
@mihaibudiu
Copy link
Copy Markdown
Contributor

You can make the point with a much smaller example.
How are the tests run?

@Karakatiza666
Copy link
Copy Markdown
Contributor Author

That's the example I was working on, did not want to spend time to provide a minimal example just for the PR.
The tests run after the Rust build is successful. I did not see a reason to make running the tests optional since if there are no tests defined the impact on the total compilation time is negligible

@Karakatiza666
Copy link
Copy Markdown
Contributor Author

Test failures are displayed in the "Errors" tab and allow navigating to the LOC of the test assertion, but do not prevent starting up the pipeline

@mihaibudiu
Copy link
Copy Markdown
Contributor

So the Rust unit tests run every time the pipeline is recompiled?

@Karakatiza666 Karakatiza666 changed the title [RFC] Add ability to declate unit tests in udf.rs [RFC] Add ability to declare unit tests in udf.rs Oct 16, 2025
@Karakatiza666
Copy link
Copy Markdown
Contributor Author

Yes

Signed-off-by: Karakatiza666 <bulakh.96@gmail.com>
@Karakatiza666
Copy link
Copy Markdown
Contributor Author

For the sake of an argument I have added a udf_checksum to avoid re-running UDF tests if UDF-related code has not changed

Signed-off-by: feldera-bot <feldera-bot@feldera.com>
@blp
Copy link
Copy Markdown
Member

blp commented Nov 14, 2025

@Karakatiza666 Is this ready for review?

@Karakatiza666
Copy link
Copy Markdown
Contributor Author

I need more feedback on this. I believe @gz had a consideration that the debug symbols would get included into compiled dependencies of a production pipeline?

Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

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

Interesting RFC! The idea of running UDF unit tests after compilation is valuable. A few architectural issues to address before this is ready.

/// Calculates a checksum only for UDF-related content (udf_rust and udf_toml).
/// This is used to determine if tests need to be re-run.
fn calculate_udf_checksum(udf_rust: &str, udf_toml: &str) -> String {
let mut hasher = sha::Sha256::new();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The UDF checksum only covers udf_rust + udf_toml, but UDF code is compiled against the SQL-generated types. If the SQL program changes (changing generated Rust interfaces), the UDF's behavior against the new types could differ — but tests won't re-run because the UDF checksum is unchanged. Either include a hash of the SQL-generated code here, or tie the test-skip logic to the overall source_checksum (which already covers everything).

if exit_status.success() {
// Retrieve previous UDF checksum to determine if tests need to be re-run
let previous_udf_checksum = if let Some(db) = db.clone() {
match db
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The compiler fetching pipeline state from the DB is an awkward inversion of responsibility. call_compiler is a low-level function; it shouldn't be querying pipeline status. The checksum comparison (previous_udf_checksum == current_udf_checksum) belongs at the call site in attempt_end_to_end_rust_compilation, where the previous checksum can be passed in as a parameter. This would also remove the db parameter from call_compiler entirely.

let mut process = command.spawn().map_err(|e| {
RustCompilationError::SystemError(
UtilError::IoError("running 'cargo test'".to_string(), e).to_string(),
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The ? here propagates RustCompilationError::SystemError (e.g., cargo binary not found, I/O error) all the way up and would block pipeline startup — contradicting the PR description that says test failures don't prevent starting the pipeline. System errors during test execution should probably be treated as "tests unavailable" rather than a hard failure. Consider catching errors here and returning a RustCompilationInfo with a non-zero exit code and the error message in stderr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants