Re: [PR] AVRO-4015: [C++] [avro]

[via GitHub]

jinxidoru commented on PR #3008:
URL: https://github.com/apache/avro/pull/3008#issuecomment-2218302999

   @Gerrit0 - Would it be possible to wise to add a cmake option to determine 
whether or not to include libfmt? Though I'll admit that I'm a bit unclear how 
you could be using the library without the libfmt dependency.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4014: [Rust] Add value and schema to ValidationWithReason error class [avro]

[via GitHub]

CodingAnarchy commented on PR #3007:
URL: https://github.com/apache/avro/pull/3007#issuecomment-2217987034

   @martin-g Unfortunately, that log doesn't get fired because `src/writer.rs` 
uses `Value#validate_internal` itself in the code that is raising the error I 
see:
   
   
https://github.com/apache/avro/blob/bb5b8429b7bec86a972f2f1f5ae5f18eeef332a7/lang/rust/avro/src/writer.rs#L538-L577
   
   Is the preference instead that I update this code in the writer to use the 
same logging format as in `Value#validate_schemata`? 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on even more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on PR #2966:
URL: https://github.com/apache/avro/pull/2966#issuecomment-2217465036

   @Fokko hopefully fixed this time! Still can't seem to reproduce this 
locally... if this still doesn't fix it, I'll propose a change to the CI 
pipeline to print out the GCC version used so I can install that.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4014: [Rust] Add value and schema to ValidationWithReason error class [avro]

[via GitHub]

martin-g commented on PR #3007:
URL: https://github.com/apache/avro/pull/3007#issuecomment-2216805489

   Please see 
https://github.com/apache/avro/blob/bb5b8429b7bec86a972f2f1f5ae5f18eeef332a7/lang/rust/avro/src/types.rs#L381-L392


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4015: [C++] [avro]

[via GitHub]

martin-g commented on PR #3008:
URL: https://github.com/apache/avro/pull/3008#issuecomment-2216795737

   I like the renaming of `api/` to `include/avro/` ! 
   But I am not sure how much breaking this change is.
   // CC @Gerrit0 @mkmkme 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Add missing ASF headers in rust files [avro]

[via GitHub]

jbonofre commented on PR #3004:
URL: https://github.com/apache/avro/pull/3004#issuecomment-2216544207

   @martin-g thanks !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Add missing ASF headers in rust files [avro]

[via GitHub]

martin-g commented on PR #3004:
URL: https://github.com/apache/avro/pull/3004#issuecomment-2216542891

   Thanks, @jbonofre !
   I've just removed them!
   Actually the config.yaml was named config.yaml.disabled, so it was not 
problematic. Anyway, it is removed now!


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Add missing ASF headers in rust files [avro]

[via GitHub]

jbonofre commented on PR #3004:
URL: https://github.com/apache/avro/pull/3004#issuecomment-2216491953

   @martin-g thanks ! Indeed I was surprised to see these files on the 1.11 
branch but not on main. Let me know if you want me to create a PR to delete 
those files. 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Add missing ASF headers in rust files [avro]

[via GitHub]

martin-g commented on PR #3004:
URL: https://github.com/apache/avro/pull/3004#issuecomment-2215305452

   Actually those files shouldn't have been committed at all!
   I am not sure when and how I added them without noticing. 
   I'll remove them as soon as I am in front of my dev machine!
   Please don't make a release with them! The config.yaml will break others' 
builds. 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on even more compiler warnings [avro]

[via GitHub]

Fokko commented on PR #2966:
URL: https://github.com/apache/avro/pull/2966#issuecomment-2214551217

   @Gerrit0 same error, different line:
   ```
   /home/runner/work/avro/avro/lang/c++/impl/Resolver.cc: In member function 
‘virtual void avro::EnumParser::parse(avro::Reader&, uint8_t*) const’:
   /home/runner/work/avro/avro/lang/c++/impl/Resolver.cc:310:16: error: useless 
cast to type ‘size_t’ {aka ‘long unsigned int’} [-Werror=useless-cast]
 310 | assert(static_cast(val) < mapping_.size());
 |^~~~
   ```


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache.hadoop:hadoop-client from 3.3.6 to 3.4.0 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2816:
URL: https://github.com/apache/avro/pull/2816#issuecomment-2212519051

   @dependabot rebase


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3968 - [Java] Support for custom @AvroNamespace annotation [avro]

[via GitHub]

henryf1991 commented on code in PR #2887:
URL: https://github.com/apache/avro/pull/2887#discussion_r1667657527


##
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java:
##
@@ -802,6 +800,21 @@ private String simpleName(Class c) {
 return simpleName;
   }
 
+  /*
+   * Function checks if there is @AvroTypeName annotation on the class. If 
present
+   * then returns the value of the annotation else returns the package of the
+   * class
+   */
+  private String getNamespace(Class c) {
+AvroTypeName avroTypeName = c.getAnnotation(AvroTypeName.class);
+if (avroTypeName != null) {
+  return avroTypeName.value();
+}
+if (c.getEnclosingClass() != null) // nested class
+  return c.getEnclosingClass().getName().replace('$', '.');

Review Comment:
   Please find my comments below
   
   1. Classes can be nested further, e.g. a class within a class within a 
top-level class. Would this method need recursion? Or should only one level of 
nesting be supported? - _To avoid any deviation I have kept the implementation 
same as the current implementation (without the namespace annotation) which 
only looks at the immediate top enclosing class for forming the namepsace._
   
   2. Without the namespace annotation, the namespace of a top-level class is 
its package, and the namespace of a nested class is package plus name of its 
enclosing class. In case the enclosing class has the namespace annotation, 
should its name also be added to the nested namespace in the same way? - _Thats 
a valid point. Happy to change if the reviewer suggests so_



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3968 - [Java] Support for custom @AvroNamespace annotation [avro]

[via GitHub]

ex-ratt commented on code in PR #2887:
URL: https://github.com/apache/avro/pull/2887#discussion_r1667628999


##
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java:
##
@@ -802,6 +800,21 @@ private String simpleName(Class c) {
 return simpleName;
   }
 
+  /*
+   * Function checks if there is @AvroTypeName annotation on the class. If 
present
+   * then returns the value of the annotation else returns the package of the
+   * class
+   */
+  private String getNamespace(Class c) {
+AvroTypeName avroTypeName = c.getAnnotation(AvroTypeName.class);
+if (avroTypeName != null) {
+  return avroTypeName.value();
+}
+if (c.getEnclosingClass() != null) // nested class
+  return c.getEnclosingClass().getName().replace('$', '.');

Review Comment:
   I'm not a reviewer, but have a few questions.
   
   1. Classes can be nested further, e.g. a class within a class within a 
top-level class. Would this method need recursion? Or should only one level of 
nesting be supported?
   2. Without the namespace annotation, the namespace of a top-level class is 
its package, and the namespace of a nested class is package plus name of its 
enclosing class. In case the enclosing class has the namespace annotation, 
should its name also be added to the nested namespace in the same way?



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Define bundler specific version working with default ruby version installed [avro]

[via GitHub]

jbonofre commented on PR #2996:
URL: https://github.com/apache/avro/pull/2996#issuecomment-2211679434

   @Fokko do you mind to take a look and merge if it's OK for you ? I'm blocked 
due to that on `branch-1.11`.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Fix PHP modules installation path [avro]

[via GitHub]

jbonofre commented on PR #2998:
URL: https://github.com/apache/avro/pull/2998#issuecomment-2210556087

   I found another issue on `main`: the PHP version installed by default now is 
8.1 (not 7.4 anymore). I'm doing another PR. Sorry about that.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

martin-g commented on PR #2954:
URL: https://github.com/apache/avro/pull/2954#issuecomment-2208969749

   Thank you, @Johnabell !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

martin-g commented on code in PR #2954:
URL: https://github.com/apache/avro/pull/2954#discussion_r1665674276


##
lang/rust/avro_derive/src/lib.rs:
##
@@ -281,6 +286,29 @@ fn type_to_schema_expr(ty: ) -> Result> {
 }
 }
 
+fn default_enum_variant(
+data_enum: ::DataEnum,
+error_span: Span,
+) -> Result, Vec> {
+match data_enum
+.variants
+.iter()
+.filter(|v| v.attrs.iter().any(is_default_attr))
+.collect::>()[..]
+{
+[] => Ok(None),
+[default] => Ok(Some(default.ident.to_string())),
+_ => Err(vec![syn::Error::new(

Review Comment:
   ```suggestion
   multiple => Err(vec![syn::Error::new(
   ```
   Could we catch here the values and print them at line 303 ?



##
lang/rust/avro_derive/src/lib.rs:
##
@@ -433,6 +461,89 @@ mod tests {
 };
 }
 
+#[test]
+fn test_basic_enum_with_default() {
+let basic_enum = quote! {
+enum Basic {
+#[default]
+A,
+B,
+C,
+D
+}
+};
+match syn::parse2::(basic_enum) {
+Ok(mut input) => {
+let derived = derive_avro_schema( input);
+assert!(derived.is_ok());
+assert_eq!(derived.unwrap().to_string(), quote! {
+impl apache_avro::schema::derive::AvroSchemaComponent for 
Basic {
+fn get_schema_in_ctxt(
+named_schemas:  std::collections::HashMap<
+apache_avro::schema::Name,
+apache_avro::schema::Schema
+>,
+enclosing_namespace: 
+) -> apache_avro::schema::Schema {
+let name = apache_avro::schema::Name::new("Basic")
+.expect(!("Unable to parse schema name 
{}", "Basic")[..])
+.fully_qualified_name(enclosing_namespace);
+let enclosing_namespace = 
+if named_schemas.contains_key() {
+apache_avro::schema::Schema::Ref { name: 
name.clone() }
+} else {
+named_schemas.insert(
+name.clone(),
+apache_avro::schema::Schema::Ref { name: 
name.clone() }
+);
+
apache_avro::schema::Schema::Enum(apache_avro::schema::EnumSchema {
+name: 
apache_avro::schema::Name::new("Basic").expect(
+!("Unable to parse enum name 
for schema {}", "Basic")[..]
+),
+aliases: None,
+doc: None,
+symbols: vec![
+"A".to_owned(),
+"B".to_owned(),
+"C".to_owned(),
+"D".to_owned()
+],
+default: Some("A".into()),
+attributes: Default::default(),
+})
+}
+}
+}
+}.to_string());
+}
+Err(error) => panic!(
+"Failed to parse as derive input when it should be able to. 
Error: {error:?}"
+),
+};
+}
+
+#[test]
+fn test_basic_enum_with_default_twice() {
+let non_basic_enum = quote! {
+enum Basic {
+#[default]
+A,
+B,
+#[default]
+C,
+D
+}
+};
+match syn::parse2::(non_basic_enum) {
+Ok(mut input) => {
+assert!(derive_avro_schema( input).is_err())

Review Comment:
   Please assert the error message here.



##
lang/rust/avro_derive/src/lib.rs:
##
@@ -433,6 +461,89 @@ mod tests {
 };
 }
 
+#[test]
+fn test_basic_enum_with_default() {
+let basic_enum = quote! {
+enum Basic {
+#[default]
+A,
+B,
+C,
+D
+}
+};
+match syn::parse2::(basic_enum) {
+Ok(mut input) => {
+let derived = derive_avro_schema( input);
+assert!(derived.is_ok());
+assert_eq!(derived.unwrap().to_string(), quote! {
+impl apache_avro::schema::derive::AvroSchemaComponent for 
Basic {
+ 

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

martin-g commented on PR #2954:
URL: https://github.com/apache/avro/pull/2954#issuecomment-2208922507

   Let me try to address those ^^


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4010: [Rust] Avoid re-resolving schema on every read() [avro]

[via GitHub]

martin-g commented on PR #2995:
URL: https://github.com/apache/avro/pull/2995#issuecomment-2206257281

   Thank you, @spektom !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4010: [Rust] Avoid re-resolving schema on every read() [avro]

[via GitHub]

martin-g commented on PR #2995:
URL: https://github.com/apache/avro/pull/2995#issuecomment-2206093834

   Please don't force push! It is hard to see what change you did to fix the 
broken build since the last time.
   At the end we do "Squash and merge" anyway so there is only one commit per 
issue/PR.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4010: [Rust] Avoid re-resolving schema on every read() [avro]

[via GitHub]

spektom commented on PR #2995:
URL: https://github.com/apache/avro/pull/2995#issuecomment-2206083719

   @martin-g Can you please review this performance improvement?


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on even more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on code in PR #2966:
URL: https://github.com/apache/avro/pull/2966#discussion_r1659426311


##
lang/c++/api/LogicalType.hh:
##
@@ -47,17 +47,17 @@ public:
 // Precision must be positive and scale must be either positive or zero. 
The
 // setters will throw an exception if they are called on any type other
 // than DECIMAL.
-void setPrecision(int precision);
-int precision() const { return precision_; }
-void setScale(int scale);
-int scale() const { return scale_; }
+void setPrecision(int64_t precision);

Review Comment:
   Fair enough, I've instead added `static_cast` to where we read this field



##
lang/c++/impl/Node.cc:
##
@@ -146,7 +146,7 @@ void Node::setLogicalType(LogicalType logicalType) {
 if (type_ == AVRO_FIXED) {
 // Max precision that can be supported by the current size of
 // the FIXED type.
-long maxPrecision = floor(log10(2.0) * (8.0 * fixedSize() - 
1));
+int32_t maxPrecision = static_cast(floor(log10(2.0) * 
(8.0 * static_cast(fixedSize()) - 1)));

Review Comment:
   @Fokko I didn't introduce this here, so I don't think I should fix it 
here... but I believe this calculation is incorrect.
   
   According to [the 
spec](https://avro.apache.org/docs/1.11.1/specification/#decimal), the maximum 
precision should be:
   
   $$
   floor(log_{10}(2^{8 \times n - 1} - 1))
   $$
   
   Rust is the only language implementation that appears to implement this 
correctly. Python, Java, and C++ all implement this as:
   
   $$
   floor(log_{10}(2^{8 \times n - 1}))
   $$
   
   JavaScript appears to have an example of a `DecimalType` with this same 
issue, but it doesn't appear to be in any library code shipped by the library.
   



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on even more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on PR #2966:
URL: https://github.com/apache/avro/pull/2966#issuecomment-2197837040

   Out of curiosity I looked at the other open PRs, I believe this supersedes 
#1852 and #2300.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3748: [Java] Fix SeekableInput.skip [avro]

[via GitHub]

opwvhk commented on code in PR #2984:
URL: https://github.com/apache/avro/pull/2984#discussion_r1657115871


##
lang/java/avro/src/main/java/org/apache/avro/file/SeekableByteArrayInput.java:
##
@@ -34,8 +35,12 @@ public long length() throws IOException {
 
   @Override
   public void seek(long p) throws IOException {
-this.reset();
-this.skip(p);
+if (p >= this.count) {
+  throw new EOFException();
+}
+if (p >= 0) {
+  this.pos = (int) p;
+}

Review Comment:
   A bit longer this way, but the original would go wrong when `mark()` was 
ever called.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache.commons:commons-compress from 1.25.0 to 1.26.0 in /lang/java [avro]

[via GitHub]

rtyley commented on PR #2758:
URL: https://github.com/apache/avro/pull/2758#issuecomment-2189499143

   Could this be change be released in a new release of `avro`?
   
   CVE-2024-25710 affects `commons-compress` from version 1.3 through 1.25.0, 
so this upgrade to 1.26.0 would be useful!


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4006: [Java] Fix block finish while reading data files [avro]

[via GitHub]

opwvhk commented on code in PR #2967:
URL: https://github.com/apache/avro/pull/2967#discussion_r1651136792


##
lang/java/avro/src/main/java/org/apache/avro/file/DataFileReader.java:
##
@@ -170,7 +171,7 @@ public void seek(long position) throws IOException {
 vin = DecoderFactory.get().binaryDecoder(this.sin, vin);
 datumIn = null;
 blockRemaining = 0;
-blockStart = position;
+blockFinished();

Review Comment:
   This has the same result, but I'm using `blockFinished()` to use the same 
code in all cases.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4006: [Java] Fix block finish while reading data files [avro]

[via GitHub]

opwvhk commented on code in PR #2967:
URL: https://github.com/apache/avro/pull/2967#discussion_r1651134309


##
lang/java/avro/src/test/java/org/apache/avro/TestDataFile.java:
##
@@ -221,6 +228,37 @@ private void testSyncDiscovery(CodecFactory codec) throws 
IOException {
 reader.seek(sync);
 assertNotNull(reader.next());
   }
+  // Lastly, confirm that reading (but not decoding) all blocks results in 
the
+  // same sync points
+  reader.sync(0);
+  ArrayList syncs2 = new ArrayList<>();
+  while (reader.hasNext()) {
+syncs2.add(reader.previousSync());
+reader.nextBlock();
+  }
+  assertEquals(syncs, syncs2);
+}
+  }
+
+  private void testReadLastRecord(CodecFactory codec) throws IOException {
+File file = makeFile(codec);
+try (DataFileReader reader = new DataFileReader<>(file, new 
GenericDatumReader<>())) {
+  long lastBlockStart = -1;
+  while (reader.hasNext()) {
+// This algorithm can be made more efficient by checking if the 
underlying
+// SeekableFileInput has been fully read: if so, the last block is in
+// memory, and calls to next() will decode it.
+// NOTE: this depends on the current implementation of DataFileReader.

Review Comment:
   Using a `SeekableFileInput` (checking if `tell()` and `length()` yield the 
same result) works even without fixing this bug, but depends on the internal 
implementation. The algorithm in the current test is cleaner in that it only 
uses the public API.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4006: [Java] Fix block finish while reading data files [avro]

[via GitHub]

opwvhk commented on code in PR #2967:
URL: https://github.com/apache/avro/pull/2967#discussion_r1651131137


##
lang/java/avro/src/test/java/org/apache/avro/TestDataFile.java:
##
@@ -221,6 +228,37 @@ private void testSyncDiscovery(CodecFactory codec) throws 
IOException {
 reader.seek(sync);
 assertNotNull(reader.next());
   }
+  // Lastly, confirm that reading (but not decoding) all blocks results in 
the
+  // same sync points
+  reader.sync(0);
+  ArrayList syncs2 = new ArrayList<>();
+  while (reader.hasNext()) {
+syncs2.add(reader.previousSync());
+reader.nextBlock();
+  }
+  assertEquals(syncs, syncs2);

Review Comment:
   This bit tests that reading/skipping blocks yields the same sync points as 
reading/decoding single records.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4006: [Java] Fix block finish while reading data files [avro]

[via GitHub]

opwvhk commented on code in PR #2967:
URL: https://github.com/apache/avro/pull/2967#discussion_r1651129635


##
doc/themes/docsy:
##


Review Comment:
   Hrm... this was not intended



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-1830 [Perl] Support containers without codec [avro]

[via GitHub]

martin-g commented on PR #2965:
URL: https://github.com/apache/avro/pull/2965#issuecomment-2186397669

   Thank you, @jjatria !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4007: [rust] Faster `is_nullable` for UnionSchema [avro]

[via GitHub]

JohnEmhoff commented on PR #2961:
URL: https://github.com/apache/avro/pull/2961#issuecomment-2186368097

   Thank you very much @martin-g ❤️


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-1830 [Perl] Support containers without codec [avro]

[via GitHub]

jjatria commented on PR #2965:
URL: https://github.com/apache/avro/pull/2965#issuecomment-2186325634

   @martin-g: I've rebased this on main and force pushed without the conflict 
:+1: 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-1830 [Perl] Support containers without codec [avro]

[via GitHub]

martin-g commented on PR #2965:
URL: https://github.com/apache/avro/pull/2965#issuecomment-2185881840

   There are conflicts in the CHANGES file but I cannot resolve them because we 
(the maintainers) do not have permissions to push to your branch.
   


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4007: [rust] Faster `is_nullable` for UnionSchema [avro]

[via GitHub]

martin-g commented on PR #2961:
URL: https://github.com/apache/avro/pull/2961#issuecomment-2185869327

   Thank you, @JohnEmhoff !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4007: [rust] Faster `is_nullable` for UnionSchema [avro]

[via GitHub]

martin-g commented on PR #2961:
URL: https://github.com/apache/avro/pull/2961#issuecomment-2185868769

   I've approved your account a few days ago but to be able to merge your 
improvement I also created AVRO-4007 for you!


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-1514: Clean up Perl dependencies [avro]

[via GitHub]

martin-g commented on PR #2962:
URL: https://github.com/apache/avro/pull/2962#issuecomment-2185859079

   Thank you, @jjatria !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [rust] Faster `is_nullable` for UnionSchema [avro]

[via GitHub]

martin-g commented on code in PR #2961:
URL: https://github.com/apache/avro/pull/2961#discussion_r1648503709


##
lang/rust/avro/src/schema.rs:
##
@@ -902,7 +902,10 @@ impl UnionSchema {
 
 /// Returns true if the any of the variants of this `UnionSchema` is 
`Null`.
 pub fn is_nullable() -> bool {
-!self.schemas.is_empty() && self.schemas.iter().any(|s| s == 
::Null)
+self.schemas.iter().any(|x| match x {
+Schema::Null => true,
+_ => false

Review Comment:
   ```suggestion
   _ => false,
   ```



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-4000 Upgrade parent pom to 32 [avro]

[via GitHub]

Fokko commented on code in PR #2955:
URL: https://github.com/apache/avro/pull/2955#discussion_r1642714743


##
pom.xml:
##
@@ -47,23 +47,19 @@
 build/avro-doc-${project.version}/api
 
 
-0.16.1

Review Comment:
   Are these renames required? People might be overriding specific properties, 
and now we change them :)



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

martin-g commented on PR #2954:
URL: https://github.com/apache/avro/pull/2954#issuecomment-2173174918

   > Looks like the CI failure is related to [this 
issue](https://github.com/TedDriggs/darling/issues/293).
   
   Please rebase to latest `main`


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

Johnabell commented on PR #2954:
URL: https://github.com/apache/avro/pull/2954#issuecomment-2173146631

   Looks like the CI failure is related to [this 
issue](https://github.com/TedDriggs/darling/issues/293).


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

Johnabell commented on PR #2954:
URL: https://github.com/apache/avro/pull/2954#issuecomment-2173138221

   > > since avro supports both default values on fields and enums
   > 
   > Someone else said the opposite ... I am not sure what to do here.
   
   Based on the [avro 
specification](https://avro.apache.org/docs/1.11.1/specification) it appears to 
be supported in both enums and record fields:
   
   1. [Enums](https://avro.apache.org/docs/1.11.1/specification/#enums)
   2. [Record 
fields](https://avro.apache.org/docs/1.11.1/specification/#schema-record)
   
   My understanding it that they have slightly different use cases:
   
   A field default mean the value of a field need not be provided. In this way 
consumers can continue to consume records based on schemas where the field is 
missing.
   
   The default value on a enum is to prevent the addition of a new variant as 
being a breaking change. In such case a client is able to use the default value 
when it encounters a variant it does not recognise. This would not be solved by 
a default on the field since the value is being provided, but is not one that 
the consumer knows about in its schema. So it would reject the message as being 
inconsistent with the schema it is using. However, when the default value is 
set on the enum it is able to use this value instead and therefore ensure 
compatibility when new variants are added.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3687 [Rust - avro_derive]: Add support for default enum values for rust derive macros [avro]

[via GitHub]

martin-g commented on PR #2954:
URL: https://github.com/apache/avro/pull/2954#issuecomment-2173106032

   > since avro supports both default values on fields and enums
   
   Someone else said the opposite ...
   I am not sure what to do here.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache:apache from 31 to 32 in /lang/java [avro]

[via GitHub]

dependabot[bot] commented on PR #2865:
URL: https://github.com/apache/avro/pull/2865#issuecomment-2171047957

   OK, I won't notify you again about this release, but will get in touch when 
a new version is available. If you'd rather skip all updates until the next 
major or minor version, let me know by commenting `@dependabot ignore this 
major version` or `@dependabot ignore this minor version`. You can also ignore 
all major, minor, or patch releases for a dependency by adding an [`ignore` 
condition](https://docs.github.com/en/code-security/supply-chain-security/configuration-options-for-dependency-updates#ignore)
 with the desired `update_types` to your config file.
   
   If you change your mind, just re-open this PR and I'll resolve any conflicts 
on it.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache:apache from 31 to 32 in /lang/java [avro]

[via GitHub]

slachiewicz commented on PR #2865:
URL: https://github.com/apache/avro/pull/2865#issuecomment-2170932442

   Superseded  by #2955


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3990 [C++] Fix invalid code generation for union with reserved name [avro]

[via GitHub]

martin-g commented on PR #2930:
URL: https://github.com/apache/avro/pull/2930#issuecomment-2167647065

   Thank you, @Gerrit0 !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

martin-g commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1639547573


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+if (trustAllPackages() || clazz.isPrimitive()) {
+  return;
+}
+
+boolean found = false;
+Package thePackage = clazz.getPackage();
+if (thePackage != null) {
+  for (String trustedPackage : getTrustedPackages()) {
+if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+throw new ClassNotFoundException("Forbidden " + clazz
++ "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");

Review Comment:
   Oh, right!
   False alarm!



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

KalleOlaviNiemitalo commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1639546634


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+if (trustAllPackages() || clazz.isPrimitive()) {
+  return;
+}
+
+boolean found = false;
+Package thePackage = clazz.getPackage();
+if (thePackage != null) {
+  for (String trustedPackage : getTrustedPackages()) {
+if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+throw new ClassNotFoundException("Forbidden " + clazz

Review Comment:
   I wonder if this should throw SecurityException rather than 
ClassNotFoundException.  I don't have a strong opinion on that.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

KalleOlaviNiemitalo commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1639540926


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+if (trustAllPackages() || clazz.isPrimitive()) {
+  return;
+}
+
+boolean found = false;
+Package thePackage = clazz.getPackage();
+if (thePackage != null) {
+  for (String trustedPackage : getTrustedPackages()) {
+if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+throw new ClassNotFoundException("Forbidden " + clazz
++ "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");

Review Comment:
   Doesn't the exception mention the class name already?  `"Forbidden " + 
clazz` includes `Class clazz` whose package was checked, and [java.lang.Class 
overrides 
toString()](https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#toString--)
 to include the fully-qualified name.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

KalleOlaviNiemitalo commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1639540926


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+if (trustAllPackages() || clazz.isPrimitive()) {
+  return;
+}
+
+boolean found = false;
+Package thePackage = clazz.getPackage();
+if (thePackage != null) {
+  for (String trustedPackage : getTrustedPackages()) {
+if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+throw new ClassNotFoundException("Forbidden " + clazz
++ "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");

Review Comment:
   Doesn't the exception mention the class name already?  `"Forbidden " + 
clazz` includes `Class clazz` whose package was checked, and [java.lang.Class 
overrides 
ToString()](https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#toString--)
 to include the fully-qualified name.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

martin-g commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1639482701


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -24,12 +24,25 @@
 import org.apache.avro.io.ResolvingDecoder;
 import org.apache.avro.util.ClassUtils;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * {@link org.apache.avro.io.DatumReader DatumReader} for generated Java
  * classes.
  */
 public class SpecificDatumReader extends GenericDatumReader {
+
+  public static final String[] SERIALIZABLE_PACKAGES;
+
+  static {
+SERIALIZABLE_PACKAGES = 
System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES",
+
"java.lang,java.math,java.io,java.net,org.apache.avro.reflect").split(",");
+  }
+
+  private List trustedPackages = new ArrayList<>();

Review Comment:
   ```suggestion
 private final List trustedPackages = new ArrayList<>();
   ```



##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));

Review Comment:
   ```suggestion
   return (trustedPackages.size() == 1 && 
"*".equals(trustedPackages.get(0)));
   ```
   



##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+if (trustAllPackages() || clazz.isPrimitive()) {
+  return;
+}
+
+boolean found = false;
+Package thePackage = clazz.getPackage();
+if (thePackage != null) {
+  for (String trustedPackage : getTrustedPackages()) {
+if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+throw new ClassNotFoundException("Forbidden " + clazz
++ "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");
+  }
+}
+  }
+
+  public List getTrustedPackages() {

Review Comment:
   This method should be `final` or it should be used at 
https://github.com/apache/avro/pull/2934/files#diff-95af304493048b22c3dda8fbfc7efb0e0c1c5e876f0234c2b8d0635bb2bcb494R127



##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
 if (name == null)
   return null;
 try {
-  return ClassUtils.forName(getData().getClassLoader(), name);
+  Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+  checkSecurity(clazz);
+  return clazz;
 } catch (ClassNotFoundException e) {
   throw new AvroRuntimeException(e);
 }
   }
 
+  private boolean trustAllPackages() {
+return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+if (trustAllPackages() || clazz.isPrimitive()) {
+  return;
+}
+
+boolean found = false;
+Package thePackage = clazz.getPackage();
+if (thePackage != null) {
+  for (String trustedPackage : getTrustedPackages()) {
+if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+  found = true;
+  break;
+}
+  }
+  if (!found) {
+throw new ClassNotFoundException("Forbidden " + clazz
++ "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");

Review Comment:
   IMO the exception should mention the problematic class name. Currently it 

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

jbonofre commented on PR #2934:
URL: https://github.com/apache/avro/pull/2934#issuecomment-2165884762

   @Fokko @martin-g @KalleOlaviNiemitalo I updated the PR. Can you guys please 
take a look ? Thanks !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3995 [C++] Require C++17 to compile Avro [avro]

[via GitHub]

martin-g commented on PR #2949:
URL: https://github.com/apache/avro/pull/2949#issuecomment-2165017455

   Thank you, @Gerrit0 !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3995 [C++] Require C++17 to compile Avro [avro]

[via GitHub]

mkmkme commented on code in PR #2949:
URL: https://github.com/apache/avro/pull/2949#discussion_r1637755772


##
lang/c++/impl/avrogencpp.cc:
##
@@ -728,28 +719,15 @@ void CodeGen::generate(const ValidSchema ) {
 os_ << "#ifndef " << h << "\n";
 os_ << "#define " << h << "\n\n\n";
 
-os_ << "#include \n";
-#if __cplusplus >= 201703L
-os_ << "#include \n";
-#else
-if (useCpp17_)
-os_ << "#include \n";
-else
-os_ << "#include \"boost/any.hpp\"\n";
-#endif
-os_ << "#include \"" << includePrefix_ << "Specific.hh\"\n"
+os_ << "#include \n"
+<< "#include \n"
+<< "#include \"" << includePrefix_ << "Specific.hh\"\n"
 << "#include \"" << includePrefix_ << "Encoder.hh\"\n"
 << "#include \"" << includePrefix_ << "Decoder.hh\"\n"
 << "\n";
 
-vector nsVector;
 if (!ns_.empty()) {
-boost::algorithm::split_regex(nsVector, ns_, boost::regex("::"));
-for (vector::const_iterator it =
- nsVector.begin();
- it != nsVector.end(); ++it) {
-os_ << "namespace " << *it << " {\n";
-}
+os_ << "namespace " << ns_ << " {\n";

Review Comment:
   C++17 [supports nested 
namespaces](https://stackoverflow.com/questions/11358425/is-there-a-better-way-to-express-nested-namespaces-in-c-within-the-header#28022457),
 so this should be fine to print as is.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3995 [C++] Require C++17 to compile Avro [avro]

[via GitHub]

martin-g commented on code in PR #2949:
URL: https://github.com/apache/avro/pull/2949#discussion_r1637606433


##
lang/c++/impl/avrogencpp.cc:
##
@@ -728,28 +719,15 @@ void CodeGen::generate(const ValidSchema ) {
 os_ << "#ifndef " << h << "\n";
 os_ << "#define " << h << "\n\n\n";
 
-os_ << "#include \n";
-#if __cplusplus >= 201703L
-os_ << "#include \n";
-#else
-if (useCpp17_)
-os_ << "#include \n";
-else
-os_ << "#include \"boost/any.hpp\"\n";
-#endif
-os_ << "#include \"" << includePrefix_ << "Specific.hh\"\n"
+os_ << "#include \n"
+<< "#include \n"
+<< "#include \"" << includePrefix_ << "Specific.hh\"\n"
 << "#include \"" << includePrefix_ << "Encoder.hh\"\n"
 << "#include \"" << includePrefix_ << "Decoder.hh\"\n"
 << "\n";
 
-vector nsVector;
 if (!ns_.empty()) {
-boost::algorithm::split_regex(nsVector, ns_, boost::regex("::"));
-for (vector::const_iterator it =
- nsVector.begin();
- it != nsVector.end(); ++it) {
-os_ << "namespace " << *it << " {\n";
-}
+os_ << "namespace " << ns_ << " {\n";

Review Comment:
   Does this still work the same way ?
   `ns_` is a String that was split by `::` in the old version of the code. Now 
it is just printed as is.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3995 [C++] Require C++17 to compile Avro [avro]

[via GitHub]

Gerrit0 commented on PR #2949:
URL: https://github.com/apache/avro/pull/2949#issuecomment-2164211263

   @Fokko done!


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3999 - Avoid warnings in Perl test suite [avro]

[via GitHub]

martin-g commented on PR #2953:
URL: https://github.com/apache/avro/pull/2953#issuecomment-2163035060

   Thank you, @jjatria !


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3995 [C++] Require C++17 to compile Avro [avro]

[via GitHub]

Fokko commented on PR #2949:
URL: https://github.com/apache/avro/pull/2949#issuecomment-2162896451

   @Gerrit0 Can you rebase now https://github.com/apache/avro/pull/2931 is in?


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.codehaus.mojo:build-helper-maven-plugin from 3.5.0 to 3.6.0 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2919:
URL: https://github.com/apache/avro/pull/2919#issuecomment-2162894537

   Thanks again for the review @slachiewicz 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3998 - Switch Perl library from JSON::XS to JSON::MaybeXS [avro]

[via GitHub]

jjatria commented on PR #2952:
URL: https://github.com/apache/avro/pull/2952#issuecomment-2162764214

   @martin-g: There's a ticket now: 
https://issues.apache.org/jira/browse/AVRO-3998


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Switch Perl library from JSON::XS to JSON::MaybeXS [avro]

[via GitHub]

martin-g commented on PR #2952:
URL: https://github.com/apache/avro/pull/2952#issuecomment-2162449444

   Approved it!


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Switch Perl library from JSON::XS to JSON::MaybeXS [avro]

[via GitHub]

jjatria commented on PR #2952:
URL: https://github.com/apache/avro/pull/2952#issuecomment-2162418876

   @martin-g I've sent in an account request :bow: 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache.maven.plugins:maven-plugin-plugin from 3.13.0 to 3.13.1 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2939:
URL: https://github.com/apache/avro/pull/2939#issuecomment-2160844406

   Thanks for the review @slachiewicz 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

jbonofre commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1634945222


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -24,12 +24,26 @@
 import org.apache.avro.io.ResolvingDecoder;
 import org.apache.avro.util.ClassUtils;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * {@link org.apache.avro.io.DatumReader DatumReader} for generated Java
  * classes.
  */
 public class SpecificDatumReader extends GenericDatumReader {
+
+  public static final String[] SERIALIZABLE_PACKAGES;
+
+  static {
+SERIALIZABLE_PACKAGES = 
System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES",
+
"java.lang,java.math,java.io,java.net,org.apache.avro.reflect").split(",");
+  }
+
+  private List trustedPackages = new ArrayList();
+  private boolean trustAllPackages = false;

Review Comment:
   it was the initial idea, but it could be confusing for users/contributors. 
Let me improve a bit this part to be clearer.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Do not escape solidus in JSON output [avro]

[via GitHub]

Gerrit0 commented on PR #2929:
URL: https://github.com/apache/avro/pull/2929#issuecomment-2156012625

   I created [AVRO-3994](https://issues.apache.org/jira/browse/AVRO-3994) to 
track this change


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Do not escape solidus in JSON output [avro]

[via GitHub]

martin-g commented on PR #2929:
URL: https://github.com/apache/avro/pull/2929#issuecomment-2155632647

   I just noticed there is no Jira ticket for this. We'll need one for the 
changelog 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3126: Add support for java records [avro]

[via GitHub]

github-advanced-security[bot] commented on code in PR #1680:
URL: https://github.com/apache/avro/pull/1680#discussion_r1630898068


##
lang/java/java17-test/src/test/java/org/apache/avro/reflect/TestJavaRecordAndCustomEncoder.java:
##
@@ -0,0 +1,237 @@
+/*
+ * 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
+ *
+ * https://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.
+ */
+
+package org.apache.avro.reflect;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.Arrays;
+
+import org.apache.avro.Schema;
+import org.apache.avro.file.DataFileStream;
+import org.apache.avro.file.DataFileWriter;
+import org.apache.avro.io.DatumReader;
+import org.apache.avro.io.Decoder;
+import org.apache.avro.io.Encoder;
+import org.junit.Test;
+
+public class TestJavaRecordAndCustomEncoder {
+
+  @Test
+  public void testRead() throws IOException {
+var in = new CustomReadWrapper(new CustomRead("hello world"));
+byte[] encoded = write(in);
+CustomReadWrapper decoded = read(encoded);
+
+assertNotNull(decoded);
+assertEquals("Fixed", decoded.field().getField());
+  }
+
+  @Test
+  public void testWrite() throws IOException {
+var in = new CustomWriteWrapper(new CustomWrite("hello world"));
+byte[] encoded = write(in);
+CustomWriteWrapper decoded = read(encoded);
+
+assertNotNull(decoded);
+assertEquals("Override", decoded.field().getField());
+  }
+
+  @Test
+  public void testWriteFiedEncoder() throws IOException {
+var in = new CustomWriteWrapperWithEncoder(new CustomWrite("hello world"));
+byte[] encoded = write(in);
+CustomWriteWrapperWithEncoder decoded = read(encoded);
+
+assertNotNull(decoded);
+assertEquals("Override2", decoded.field().getField());
+  }
+
+  @Test
+  public void testReadFiedEncoder() throws IOException {
+var in = new CustomReadWrapperWithEncoder(new CustomRead("hello world"));
+byte[] encoded = write(in);
+CustomReadWrapperWithEncoder decoded = read(encoded);
+
+assertNotNull(decoded);
+assertEquals("Fixed2", decoded.field().getField());
+  }
+
+  private  T read(byte[] toDecode) throws IOException {
+DatumReader datumReader = new ReflectDatumReader<>();
+try (DataFileStream dataFileReader = new DataFileStream<>(new 
ByteArrayInputStream(toDecode, 0, toDecode.length),
+datumReader);) {
+  dataFileReader.hasNext();
+  return dataFileReader.next();
+}
+  }
+
+  private  byte[] write(T custom) {
+Schema schema = ReflectData.get().getSchema(custom.getClass());
+ReflectDatumWriter datumWriter = new ReflectDatumWriter<>();
+try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
+DataFileWriter writer = new DataFileWriter<>(datumWriter)) {
+  writer.create(schema, baos);
+  writer.append(custom);
+  writer.flush();
+  return baos.toByteArray();
+} catch (IOException e) {
+  throw new UncheckedIOException(e);
+}
+  }
+
+  @AvroEncode(using = CustomEncoderWrite.class)
+  public static class CustomWrite {
+
+private final String field;
+
+public CustomWrite(String field) {
+  this.field = field;
+}
+
+public String getField() {
+  return field;
+}
+  }
+
+  @AvroEncode(using = CustomEncoderRead.class)
+  public static class CustomRead {
+
+private final String field;
+
+public CustomRead(String field) {
+  this.field = field;
+}
+
+public String getField() {
+  return field;
+}
+  }
+
+  public static class Custom {
+
+private final String field;
+
+public Custom(String field) {
+  this.field = field;
+}
+
+public String getField() {
+  return field;
+}
+  }
+
+  public static record CustomReadWrapper(CustomRead field) {
+  }
+
+  public static record CustomWriteWrapper(CustomWrite field) {
+  }
+
+  public static record CustomReadWrapperWithEncoder(@AvroEncode(using = 
CustomEncoderRead2.class) CustomRead field) {
+  }
+
+  public static record CustomWriteWrapperWithEncoder(@AvroEncode(using = 
CustomEncoderWrite2.class) CustomWrite field) {
+  }
+
+  public 

Re: [PR] AVRO-3126: Add support for java records [avro]

[via GitHub]

martin-g commented on PR #1680:
URL: https://github.com/apache/avro/pull/1680#issuecomment-2154370712

   @ashley-taylor AFAIK 1.12.0 is around the corner, so I'd recommend to create 
your second PR on top of this one and ask for review at d...@avro.apache.org
   There were some talks about bumping Java to 17+ for 1.12.0 (it was bumped 
from 8 to 11 recently) but I didn't see any dicsussions/votes/commits about it.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3126: Add support for java records [avro]

[via GitHub]

ashley-taylor commented on PR #1680:
URL: https://github.com/apache/avro/pull/1680#issuecomment-2153533705

   @martin-g I have updated this PR with changes made since it was opened. Are 
there release plans for 1.12.0? Is there anything I can do to help merge this?
   I have a polymorphic and performance improvement PR I would like to raise 
that build on this one.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump strum_macros from 0.26.2 to 0.26.3 in /lang/rust [avro]

[via GitHub]

dependabot[bot] commented on PR #2941:
URL: https://github.com/apache/avro/pull/2941#issuecomment-2150627040

   Superseded by #2944.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

KalleOlaviNiemitalo commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1625753931


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##
@@ -24,12 +24,26 @@
 import org.apache.avro.io.ResolvingDecoder;
 import org.apache.avro.util.ClassUtils;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * {@link org.apache.avro.io.DatumReader DatumReader} for generated Java
  * classes.
  */
 public class SpecificDatumReader extends GenericDatumReader {
+
+  public static final String[] SERIALIZABLE_PACKAGES;
+
+  static {
+SERIALIZABLE_PACKAGES = 
System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES",
+
"java.lang,java.math,java.io,java.net,org.apache.avro.reflect").split(",");
+  }
+
+  private List trustedPackages = new ArrayList();
+  private boolean trustAllPackages = false;

Review Comment:
   `trustAllPackages` is never changed to `true` in the Java code.  Is it 
intended to be set with a debugger?



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3966: [Java] Fix default value serialisation for fixed and bytes [avro]

[via GitHub]

mitasov-ra commented on PR #2823:
URL: https://github.com/apache/avro/pull/2823#issuecomment-2146926822

   @Fokko can you please take a look at this PR, especially on "DISCUSSION IS 
NEEDED" part


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3826: Common tests for C++ module [avro]

[via GitHub]

martin-g commented on PR #2431:
URL: https://github.com/apache/avro/pull/2431#issuecomment-2145039994

   IMO the best documentation would be a build flag / linter that will complain 
when a newer feature is used.
   I do not use the C++ SDK but I think it is OK to use C++17 for Avro 12.x.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3826: Common tests for C++ module [avro]

[via GitHub]

Gerrit0 commented on PR #2431:
URL: https://github.com/apache/avro/pull/2431#issuecomment-2144954421

   Personally, I'd be happy to see Avro move to a C++17 minimum, so long as it 
was actually documented somewhere! The last time I updated my Avro dependency, 
it was on C++11, https://github.com/apache/avro/pull/960 changed the default 
compiler flags to 17, but didn't actually introduce any code that required 
it... 


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache.maven.plugins:maven-shade-plugin from 3.5.3 to 3.6.0 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2936:
URL: https://github.com/apache/avro/pull/2936#issuecomment-2144756991

   @dependabot rebase


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

jbonofre commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1623881900


##
lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java:
##
@@ -40,6 +41,17 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class TestReflectData {
+
+  @Test
+  void trustedPackages() throws Exception {
+Schema schema = new Schema.Parser().parse(new 
File("target/test-classes/schema-trusted-packages.json"));
+
+Schema.Field field = schema.getField("value");
+System.out.println(field);
+
+System.out.println(schema);

Review Comment:
   It should be removed (my bad sorry about that). I will clean up. 



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

jbonofre commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1623880966


##
lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java:
##
@@ -1171,6 +1171,7 @@ public static class NullableStringable {
 
   @Test
   void nullableStringableField() throws Exception {
+System.setProperty("org.apache.avro.TRUSTED_PACKAGES", "*");

Review Comment:
   Actually i think we don't need it with the default trusted packages. Let me 
double check. Thanks !



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3826: Common tests for C++ module [avro]

[via GitHub]

martin-g commented on PR #2431:
URL: https://github.com/apache/avro/pull/2431#issuecomment-2144430183

   @Gerrit0 I am not aware of any restrictions about the C++ standard that is 
(not) allowed to be used in Avro.
   If C++-14 is the recommended maximum then maybe `-std=c++14` should be added 
to the compilation settings ?!


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

martin-g commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1623875468


##
lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java:
##
@@ -1171,6 +1171,7 @@ public static class NullableStringable {
 
   @Test
   void nullableStringableField() throws Exception {
+System.setProperty("org.apache.avro.TRUSTED_PACKAGES", "*");

Review Comment:
   Should this be reverted in a `finally` ?
   Because currently it will leak to other tests too.



##
lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java:
##
@@ -40,6 +41,17 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class TestReflectData {
+
+  @Test
+  void trustedPackages() throws Exception {
+Schema schema = new Schema.Parser().parse(new 
File("target/test-classes/schema-trusted-packages.json"));
+
+Schema.Field field = schema.getField("value");
+System.out.println(field);
+
+System.out.println(schema);

Review Comment:
   This test looks like a playground. 
   It should be either finished (e.g. by using assertions) or removed.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3985: Add trusted packages support in SpecificData [avro]

[via GitHub]

Fokko commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1623649255


##
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java:
##
@@ -372,6 +372,7 @@ private static boolean isBlank(CharSequence cs) {
 
   /** Return the class that implements a schema, or null if none exists. */
   public Class getClass(Schema schema) {
+

Review Comment:
   Let's avoid unrelated changes:
   ```suggestion
   ```



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3991 [C++] Fix undefined behavior in Compiler.cc [avro]

[via GitHub]

KalleOlaviNiemitalo commented on PR #2933:
URL: https://github.com/apache/avro/pull/2933#issuecomment-2143284028

   If the `&` is removed (even though that change is not required for 
correctness), then I'd like to keep the `const` for clarity.  And the change 
won't need to be backported to branches other than `main`.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3991 [C++] Fix undefined behavior in Compiler.cc [avro]

[via GitHub]

Gerrit0 commented on PR #2933:
URL: https://github.com/apache/avro/pull/2933#issuecomment-2143276262

   How did I not know this?! Well, I feel rather dumb now.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3991 [C++] Fix undefined behavior in Compiler.cc [avro]

[via GitHub]

KalleOlaviNiemitalo commented on PR #2933:
URL: https://github.com/apache/avro/pull/2933#issuecomment-2143265569

   That's not undefined behaviour.  The lifetime of the temporary string is 
extended to match the lifetime of the reference that is initialised with it.  



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3826: Common tests for C++ module [avro]

[via GitHub]

Gerrit0 commented on PR #2431:
URL: https://github.com/apache/avro/pull/2431#issuecomment-2143213988

   @clesaec @martin-g Was it an intentional change that this introduced the use 
of `#include `, which is C++17 only, so it is no longer possible to 
compile Avro (or at least the tests) with C++14?
   
   There is a fair amount of code in Avro which is written in a way that's 
obviously pre-C++17. If it is official policy that C++14 is no longer 
supported, there are quite a few cleanup things I'd like to do, and if that's 
not the case, it needs to be documented somewhere, and some pipeline added to 
make sure it doesn't get broken again.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Fix compiler warnings in code generated by schema with empty record [avro]

[via GitHub]

Gerrit0 commented on PR #2927:
URL: https://github.com/apache/avro/pull/2927#issuecomment-2143040650

   Took another look at the validating encoder, and was able to figure out what 
the problem was from the JsonEncoder, one line fix, so I just added it here.
   
   I also found a bug with my change because of ResolvingDecoder, so fixed that.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on PR #2931:
URL: https://github.com/apache/avro/pull/2931#issuecomment-2142680767

   I've also turned on -Wconversion, will make a PR for [that 
branch](https://github.com/Gerrit0/avro/tree/wconversion) after this one is 
merged as it builds on this


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3968 - [Java] Support for custom @AvroNamespace annotation [avro]

[via GitHub]

opwvhk commented on PR #2887:
URL: https://github.com/apache/avro/pull/2887#issuecomment-2142550055

   Although I really prefer option 1, I can also live with option 2. Can you 
please make the changes for `@AvroName`, `@AvroNamespace` and the `java-class` 
property?


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3987 [Java] Concurrency improvement for ReflectData FieldAccessors [avro]

[via GitHub]

opwvhk commented on code in PR #2900:
URL: https://github.com/apache/avro/pull/2900#discussion_r1622618815


##
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java:
##
@@ -379,12 +379,14 @@ private ClassAccessorData(Class c) {
  * Return the field accessors as an array, indexed by the field index of 
the
  * given schema.
  */
-private synchronized FieldAccessor[] getAccessorsFor(Schema schema) {
-  // if synchronized is removed from this method, adjust bySchema 
appropriately
+private FieldAccessor[] getAccessorsFor(Schema schema) {
+  // to avoid synchronization, we replace the map for each modification
   FieldAccessor[] result = bySchema.get(schema);
   if (result == null) {
 result = createAccessorsFor(schema);
+Map bySchema = new 
WeakHashMap<>(this.bySchema);
 bySchema.put(schema, result);
+this.bySchema = bySchema;

Review Comment:
   The remark about `volatile` is valid: that's exactly what it does.
   
   Also a few notes about `WeakHashMap`:
   
   1. it has weak keys, so it'll only cleanup entries that can no longer be 
looked up
   2. its internal hash table is a singly-linked list, and the update to remove 
an entry is an atomic operation (that never yields an invalid data structure)
   3. the update happens without a memory barrier, which means that the worst 
case scenario is that the update is not yet available to the other thread, 
which means that the entry to be removed is read & skipped over in the 
concurrent call to `get`
   
   In practical terms, this means that because the we update a *copy* of the 
`WeakHashMap` before it becomes available, and it's only modified to expunge 
stale entries after it becomes available, the result is effectively thread safe.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on code in PR #2931:
URL: https://github.com/apache/avro/pull/2931#discussion_r1622612920


##
lang/c++/test/CommonsSchemasTests.cc:
##
@@ -45,8 +45,7 @@ void testCommonSchema(const std::filesystem::path _path) {
 DataFileWriter writer(outputDataFile.c_str(), schema);
 
 while (reader.read(datum)) {
-avro::GenericRecord  = datum.value();
-BOOST_CHECK(rec.fieldCount() >= 0);

Review Comment:
   This check is useless as `fieldCount()` returns an unsigned value



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on code in PR #2931:
URL: https://github.com/apache/avro/pull/2931#discussion_r1622603716


##
lang/c++/test/DataFileTests.cc:
##
@@ -217,7 +217,7 @@ class DataFileTest {
 #endif
 
 void testWriteWithCodec(avro::Codec codec) {
-avro::DataFileWriter df(filename, writerSchema, 100);
+avro::DataFileWriter df(filename, writerSchema, 100, 
codec);

Review Comment:
   Look at that! A place where an unused parameter warning actually revealed a 
bug in the tests, not always just noise :)



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on code in PR #2931:
URL: https://github.com/apache/avro/pull/2931#discussion_r1622602372


##
lang/c++/test/CodecTests.cc:
##
@@ -25,7 +25,7 @@
 #include "Specific.hh"
 #include "ValidSchema.hh"
 
-#include 
+#include 

Review Comment:
   Recent versions of boost have effectively deprecated this header, we don't 
use placeholders, so no additional changes are required:
   
   ```
   In file included from /usr/include/boost/bind.hpp:30,
from 
/home/gerrit/Desktop/avro/lang/c++/test/CodecTests.cc:28:
   /usr/include/boost/bind.hpp:36:1: note: ‘#pragma message: The practice of 
declaring the Bind placeholders (_1, _2, ...) in the global namespace is 
deprecated. Please use  + using namespace 
boost::placeholders, or define BOOST_BIND_GLOBAL_PLACEHOLDERS to retain the 
current behavior.’
  36 | BOOST_PRAGMA_MESSAGE(
 | ^~~~
   ```



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on code in PR #2931:
URL: https://github.com/apache/avro/pull/2931#discussion_r1622603064


##
lang/c++/test/CodecTests.cc:
##
@@ -594,7 +594,6 @@ struct TestData4 {
 const char *readerCalls;
 const char *readerValues[100];
 unsigned int depth;
-size_t recordCount;

Review Comment:
   This field was never used, and not always initialized



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Turn on more compiler warnings [avro]

[via GitHub]

Gerrit0 commented on code in PR #2931:
URL: https://github.com/apache/avro/pull/2931#discussion_r1622600509


##
lang/c++/impl/Compiler.cc:
##
@@ -286,8 +286,9 @@ static Field makeField(const Entity , SymbolTable , 
const string ) {
 const Object  = e.objectValue();
 string n = getStringField(e, m, "name");
 vector aliases;
-if (containsField(m, "aliases")) {
-for (const auto  : getArrayField(e, m, "aliases")) {
+string aliasesName = "aliases";
+if (containsField(m, aliasesName)) {
+for (const auto  : getArrayField(e, m, aliasesName)) {

Review Comment:
   I'll admit this is kind of a weird change... the compiler appears to think 
that the lifetime of the name passed to `getArrayField` may be linked to the 
lifetime of the returned reference. It doesn't appear to be from my 
investigation, but this happens to slightly improve performance wherever we use 
`containsField` + `getArrayField` together anyways, so seems a minor cost to 
pay.
   
   ```text
   /home/gerrit/Desktop/avro/lang/c++/impl/Compiler.cc: In function 
‘avro::Field avro::makeField(const json::Entity&, SymbolTable&, const 
std::string&)’:
   /home/gerrit/Desktop/avro/lang/c++/impl/Compiler.cc:290:63: error: possibly 
dangling reference to a temporary [-Werror=dangling-reference]
 290 | for (const auto  : getArrayField(e, m, "aliases")) {
 |   ^
   /home/gerrit/Desktop/avro/lang/c++/impl/Compiler.cc:290:47: note: the 
temporary was destroyed at the end of the full expression 
‘avro::getArrayField((* & e), (* & m), std::__cxx11::basic_string(((const 
char*)"aliases"), std::allocator()))’
 290 | for (const auto  : getArrayField(e, m, "aliases")) {
 |  ~^
   ```



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Fix compiler warnings in code generated by schema with empty record [avro]

[via GitHub]

Gerrit0 commented on PR #2927:
URL: https://github.com/apache/avro/pull/2927#issuecomment-2142255606

   @Fokko added!
   
   There appears to be a problem with `validatingEncoder`, I believe it is 
incorrectly rejecting my test object I left a FIXME comment to avoid 
increasing the size of this more than necessary.
   
   The union typedef feature is awesome, I wish I had it in the version of avro 
I  have at work, unfortunately it didn't add a typedef for the union within the 
array... I added some code to do this, let me know if you'd prefer that be 
pulled into a separate PR.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.codehaus.mojo:build-helper-maven-plugin from 3.5.0 to 3.6.0 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2919:
URL: https://github.com/apache/avro/pull/2919#issuecomment-2142206287

   @dependabot rebase


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache.hadoop:hadoop-client from 3.3.6 to 3.4.0 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2816:
URL: https://github.com/apache/avro/pull/2816#issuecomment-2142205755

   @dependabot rebase


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] Bump org.apache.commons:commons-compress from 1.26.1 to 1.26.2 in /lang/java [avro]

[via GitHub]

Fokko commented on PR #2920:
URL: https://github.com/apache/avro/pull/2920#issuecomment-2141384317

   @dependabot rebase


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] [C++] Fix compiler warnings in code generated by schema with empty record [avro]

[via GitHub]

Fokko commented on PR #2927:
URL: https://github.com/apache/avro/pull/2927#issuecomment-2141337335

   @Gerrit0 Thanks for adding this. Could you add a test as well?


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3987 [Java] Concurrency improvement for ReflectData FieldAccessors [avro]

[via GitHub]

KalleOlaviNiemitalo commented on code in PR #2900:
URL: https://github.com/apache/avro/pull/2900#discussion_r1618425234


##
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java:
##
@@ -379,12 +379,14 @@ private ClassAccessorData(Class c) {
  * Return the field accessors as an array, indexed by the field index of 
the
  * given schema.
  */
-private synchronized FieldAccessor[] getAccessorsFor(Schema schema) {
-  // if synchronized is removed from this method, adjust bySchema 
appropriately
+private FieldAccessor[] getAccessorsFor(Schema schema) {
+  // to avoid synchronization, we replace the map for each modification
   FieldAccessor[] result = bySchema.get(schema);
   if (result == null) {
 result = createAccessorsFor(schema);
+Map bySchema = new 
WeakHashMap<>(this.bySchema);
 bySchema.put(schema, result);
+this.bySchema = bySchema;

Review Comment:
   I'm not really a Java programmer but the `this.bySchema = bySchema` 
assignment seems OK, assuming that the `volatile` causes a memory barrier that 
ensures the effects of `bySchema.put(schema, result)` will be seen by other 
threads that read `this.bySchema` after the assignment.
   
   However, I wonder about `bySchema.get(schema)`.  Is WeakHashMap.get formally 
documented to be safe for concurrent calls?  Each time WeakHashMap.get is 
called, WeakHashMap.expungeStaleEntries can delete some entries, and although 
WeakHashMap.expungeStaleEntries synchronizes those deletions against each 
other, it doesn't seem to synchronize them against any reads that 
WeakHashMap.get may be doing on other threads.



-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3987 [Java] Concurrency improvement for ReflectData FieldAccessors [avro]

[via GitHub]

martin-g commented on PR #2900:
URL: https://github.com/apache/avro/pull/2900#issuecomment-2136679307

   > @martin-g anything you need to get this merged?
   
   I'll leave it to a Java developer because I am not that familiar with the 
Java SDK.


-- 
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-unsubscr...@avro.apache.org

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

Re: [PR] AVRO-3987 [Java] Concurrency improvement for ReflectData FieldAccessors [avro]

[via GitHub]

ashley-taylor commented on PR #2900:
URL: https://github.com/apache/avro/pull/2900#issuecomment-2136130880

   @martin-g anything you need to get this merged? 


-- 
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-unsubscr...@avro.apache.org

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