datafuselabs / databend

๐——๐—ฎ๐˜๐—ฎ, ๐—”๐—ป๐—ฎ๐—น๐˜†๐˜๐—ถ๐—ฐ๐˜€ & ๐—”๐—œ. Modern alternative to Snowflake. Cost-effective and simple for massive-scale analytics. https://databend.com
https://docs.databend.com
Other
7.29k stars 701 forks source link

Inconsistency between code and comment #15390

Closed YichiZhang0613 closed 2 weeks ago

YichiZhang0613 commented 2 weeks ago

Similar to #15162, I found some inconsistency between code and comment. In databend-main/src/common/arrow/src/arrow/array/union/mod.rs๏ผŒcomment should be "offset + length > self.len()"

/// # Panic
    /// This function panics iff `offset + length >= self.len()`.
    #[inline]
    pub fn slice(&mut self, offset: usize, length: usize) {
        assert!(
            offset + length <= self.len(),
            "the offset of the new array cannot exceed the existing length"
        );
        unsafe { self.slice_unchecked(offset, length) }
    }

In databend-main/src/common/arrow/src/arrow/array/primitive/mutable.rs, comment should be "index is larger than or equal to `self.len()"

/// # Panic
    /// Panics iff index is larger than `self.len()`.
    pub fn set(&mut self, index: usize, value: Option<T>) {
        assert!(index < self.len());

In databend-main/src/common/arrow/src/arrow/buffer/immutable.rs, maybe the comment should be "Panics iff offset + length is larger than len"

/// # Panics
    /// Panics iff `offset` is larger than `len`.
    #[inline]
    pub fn slice(&mut self, offset: usize, length: usize) {
        assert!(
            offset + length <= self.len(),
            "the offset of the new Buffer cannot exceed the existing length"
        );
        // Safety: we just checked bounds
        unsafe { self.slice_unchecked(offset, length) }
    }

and some unchecked boundary or empty constraints which are indicated in comment while aren't checked in code.For example, in databend-main/src/common/arrow/src/arrow/io/ipc/read/file.rs, the code should check whether index is out of bounds before using it directly.


/// # Panics
/// This function panics iff `index >= metadata.blocks.len()`
#[allow(clippy::too_many_arguments)]
pub fn read_batch<R: Read + Seek>(
    reader: &mut R, dictionaries: &Dictionaries, metadata: &FileMetadata, projection: Option<&[usize]>, limit: Option<usize>, index: usize, message_scratch: &mut Vec<u8>, data_scratch: &mut Vec<u8>,) -> Result<Chunk<Box<dyn Array>>> {
    let block = metadata.blocks[index];
```rust
...