DynFields, DynMethods code cleanup

[findepi]

No description provided.

[nastra]

can you elaborate please why these are being deprecated? It's not obvious for a reviewer

[findepi]

The Dyn* classs are copied from Parquet and contain more code than needed by Iceberg codebase. I assume these utilities were not meant to be API of the project and so any dead code here should be removed. Per revapi checks, removal requires prior deprecation.

[Fokko]

I don't mind having a few additional methods around. They come in a pair of {invoke, invokeChecked}. It looks weird if they are sometime defined, and sometimes missing.

[findepi]

My thinking is that any variant needed in the future is very easy to add when needed and doesn't necessitate keeping current-dead-code around. Removing dead code clears the picture and open eyes on further simplifications. Does this make sense? BTW recently, i was starring at a pretty complex class that was negatively impacted by some other changes only to eventually realize it's used only in its own tests and can simply be deleted. What a relief.

[findepi]

I still think we should just remove the dead code. But I am also ok leaving it around if it helps something. let me know which direction you want this to go

[Fokko]

I like to either remove stuff or keep it there to keep the full picture. If we decide to remove this, I think we can clean it up even further. As part of #10542 we can mark invokeChecked as deprecated there. Then we can just inline invokeChecked in DynMethods:

iceberg/common/src/main/java/org/apache/iceberg/common/DynMethods.java

Lines 54 to 68 in 91fbcaa

	@SuppressWarnings("unchecked")
	public <R> R invokeChecked(Object target, Object... args) throws Exception {
	try {
	if (argLength < 0) {
	return (R) method.invoke(target, args);
	} else {
	return (R) method.invoke(target, Arrays.copyOfRange(args, 0, argLength));
	}
	} catch (InvocationTargetException e) {
	Throwables.propagateIfInstanceOf(e.getCause(), Exception.class);
	Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class);
	throw Throwables.propagate(e.getCause());
	}
	}

Less is more!

[findepi]

i'm not sure i follow. for now i just dropped deprecation of invokeChecked methods in this PR. My intuitive reaction to IDE warning about dead code such as unused method is to remove it, but clearly it's not as simple. Maybe we do this as a follow-up, if needed.

[Fokko]

What I'm trying to say is that we should mark this invokeChecked as deprecated as well. Since that's only being used from invoke. This way we can remove invokeChecked completely from the public API.

[findepi]

you're right. done!

[Fokko]

Nice one @findepi Thanks for cleaning this up 👍