JingsongLi commented on code in PR #1238: URL: https://github.com/apache/paimon-rust/pull/1238#discussion_r4402714377
########## bindings/python/src/temporal.rs: ########## @@ -0,0 +1,296 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Batch temporal matching over sorted, non-null int64 keys. + +use arrow::array::{Array, ArrayData, Int64Array}; +use arrow::pyarrow::{FromPyArrow, ToPyArrow}; +use arrow::record_batch::RecordBatch; +use pyo3::exceptions::PyValueError; +use pyo3::prelude::*; +use std::sync::Arc; + +fn lower(times: &[i64], target: i128, mut start: usize, mut end: usize) -> usize { + while start < end { + let middle = start + (end - start) / 2; + if (times[middle] as i128) < target { + start = middle + 1; + } else { + end = middle; + } + } + start +} + +fn upper(times: &[i64], target: i128, mut start: usize, mut end: usize) -> usize { + while start < end { + let middle = start + (end - start) / 2; + if (times[middle] as i128) <= target { + start = middle + 1; + } else { + end = middle; + } + } + start +} + +#[derive(Clone, Copy)] +enum Mode { + Nearest, + Backward, + Forward, + Interpolate, + Window, +} + +struct Options { + mode: Mode, + exact: bool, + tolerance: Option<i64>, + preceding: i64, + following: i64, + left_closed: bool, + right_closed: bool, +} + +fn match_one( + times: &[i64], + target: i64, + start: usize, + end: usize, + options: &Options, +) -> (i64, i64) { + if start == end { + return (-1, -1); + } + let target = target as i128; + if matches!(options.mode, Mode::Window) { + let left = target - options.preceding as i128; + let right = target + options.following as i128; + let first = if options.left_closed { + lower(times, left, start, end) + } else { + upper(times, left, start, end) + }; + let last = if options.right_closed { + upper(times, right, start, end) + } else { + lower(times, right, start, end) + }; + return if last > first { + (first as i64, last as i64) + } else { + (0, 0) + }; + } + let left = lower(times, target, start, end); + let right = upper(times, target, left, end); + if matches!(options.mode, Mode::Interpolate) { + if left < end && times[left] as i128 == target { + return ((right - 1) as i64, (right - 1) as i64); + } + if left > start + && left < end + && options.tolerance.is_none_or(|tolerance| { + (target - times[left - 1] as i128).max(times[left] as i128 - target) + <= tolerance as i128 + }) + { + return ((left - 1) as i64, left as i64); + } + return (-1, -1); + } + let before = if options.exact { right } else { left }; + let after = if options.exact { left } else { right }; + let position = match options.mode { + Mode::Backward => (before > start).then(|| before - 1), + Mode::Forward => (after < end).then_some(after), + _ if options.exact && left < end && times[left] as i128 == target => Some(right - 1), + _ if left == start => (after < end).then_some(after), + _ if after == end || target - times[left - 1] as i128 <= times[after] as i128 - target => { + Some(left - 1) + } + _ => Some(after), + }; + match position { + Some(position) + if options.tolerance.is_none_or(|tolerance| { + (times[position] as i128 - target).abs() <= tolerance as i128 + }) => + { + (position as i64, -1) + } + _ => (-1, -1), + } +} + +fn int64_array(value: &Bound<'_, PyAny>) -> PyResult<Int64Array> { + let data = ArrayData::from_pyarrow_bound(value)?; + if data.data_type() != &arrow::datatypes::DataType::Int64 || data.null_count() != 0 { + return Err(PyValueError::new_err( + "Expected a non-null int64 Arrow array", + )); + } + Ok(Int64Array::from(data)) Review Comment: **[P2] Align foreign buffers before constructing `Int64Array`** `ArrayData::from_pyarrow_bound()` preserves the foreign buffer address, but `Int64Array::from(data)` requires 8-byte alignment and panics when it is missing. PyArrow permits valid, sorted, non-null int64 arrays backed by unaligned buffers, including zero-copy NumPy inputs. I reproduced a `PanicException` with the unchanged temporal binding using Arrow 58.4.0: ```python import struct import pyarrow as pa from pypaimon_rust import temporal_match raw = b'x' + struct.pack('<3q', 1, 3, 5) times = pa.Array.from_buffers( pa.int64(), 3, [None, pa.py_buffer(raw).slice(1)]) times.validate(full=True) # Valid, sorted, non-null int64 input temporal_match( times, pa.array([1], type=pa.int64()), pa.array([0], type=pa.int64()), pa.array([3], type=pa.int64()), 'nearest') ``` This should return `first=[0], second=[-1]`, but instead raises `PanicException: Memory pointer from external source ... is not aligned with the specified scalar type`. Please call `data.align_buffers()` before constructing the typed array, as the existing `arrow_input::array_from_pyarrow` helper already does, and add a regression test for unaligned input. The same helper imports all four input arrays, so this affects more than just `times`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
