[I] RecordBuilder should confirm non-nullable fields do not contain nullable rows [arrow-go]

["albertlockett (via GitHub)" <gi...@apache.org>]

albertlockett opened a new issue, #372:
URL: https://github.com/apache/arrow-go/issues/372

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   I am able to construct an arrow record, where a field should be non-nullable, but it contains nullable rows. I'm wondering if maybe `RecordBuilder.NewRecord()` should confirm that the fields's arrays do not contain nulls if the field is specified as `Nullable: false` in the schema?
   
   ```go
   package main
   
   import (
   	"log"
   
   	"github.com/apache/arrow-go/v18/arrow"
   	"github.com/apache/arrow-go/v18/arrow/array"
   	"github.com/apache/arrow-go/v18/arrow/memory"
   )
   
   func main() {
   	allocator := memory.NewGoAllocator()
   	schema := arrow.NewSchema([]arrow.Field{
   	        // should be not null:
   		{Name: "a", Type: arrow.PrimitiveTypes.Int32, Nullable: false}, 
   	}, nil)
   	recordBuilder := array.NewRecordBuilder(allocator, schema)
   
   	recordBuilder.Field(0).(*array.Int32Builder).Append(1)
   	recordBuilder.Field(0).(*array.Int32Builder).Append(2)
   	recordBuilder.Field(0).AppendNull()
   	recordBuilder.Field(0).(*array.Int32Builder).Append(4)
   
   	record := recordBuilder.NewRecord() // this works
   	defer record.Release()
   
   	log.Printf("record: %v", record)
   	/* prints
   2025/05/08 20:32:47 record: record:
   schema:
     fields: 1
       - a: type=int32
     rows: 4
     col[0][a]: [1 2 (null) 4]
     */
   }
   ```
   
   This is not allowed in the Rust implementation:
   ```rs
   use std::sync::Arc;
   
   use arrow_array::{Int32Array, RecordBatch};
   use arrow_schema::{DataType, Field, Schema};
   
   #[tokio::main]
   async fn main() {
       let schema = Arc::new(Schema::new(vec![Field::new("a", DataType::Int32, false)]));
       let mut ids = Int32Array:: builder(4);
       ids.append_value(1);
       ids.append_value(2);
       ids.append_null();
       ids.append_value(4);
       let ids = ids.finish();
       // this panics:
       let batch = RecordBatch::try_new(schema.clone(), vec![Arc::new(ids)]).unwrap(); 
       println!("batch: {:?}", batch);
   }
   ```
   
   
   ### Component(s)
   
   Other


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] RecordBuilder should confirm non-nullable fields do not contain null rows when building Record [arrow-go]

["zeroshade (via GitHub)" <gi...@apache.org>]

zeroshade commented on issue #372:
URL: https://github.com/apache/arrow-go/issues/372#issuecomment-2908079808

   Are you suggesting this should be done for all Nested builders? Or just for the record builder and validating only the top level of columns?
   
   My concern on this is when dealing with a non-nullable field within a nullable struct type. Adding null elements to the parent struct array necessitates adding nulls to a child which would be technically marked as non-null (or would require changing the implementation to notice this situation and to just add empty/zero values instead) causing existing code that is technically correct to potentially fail if we add this check to all nested builders. But if this check is *only* for the top level columns of a record batch, that wouldn't be a concern.
   
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org