RussellSpitzer commented on code in PR #9546:
URL: https://github.com/apache/iceberg/pull/9546#discussion_r1493066170
##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java:
##########
@@ -59,7 +61,7 @@
*/
public class HadoopTableOperations implements TableOperations {
private static final Logger LOG =
LoggerFactory.getLogger(HadoopTableOperations.class);
- private static final Pattern VERSION_PATTERN =
Pattern.compile("v([^\\.]*)\\..*");
+ private static final Pattern VERSION_PATTERN =
Pattern.compile("v([^.]*)\\..*");
Review Comment:
Again, I would urge you to separate changes that are unrelated to the PR
into other PR's. This is also not redundant and the IDE was incorrect. There is
a semantic difference between these two changes, it may not have been intended
to originally behave this way but we can't change behaviors here without a
really good reason unless we are making a change in a major version.
This probably was a bug in the original Regex but before the pattern was
> scala> val old_pattern = Pattern.compile("v([^\\.]*)\\..*");
> val old_pattern: java.util.regex.Pattern = v([^\.]*)\..*
> v followed by any number of non '\\' or '.' characters then followed by a
'.' and then any additional characters
Now it is
> scala> val new_pattern = Pattern.compile("v([^.]*)\\..*")
> val new_pattern: java.util.regex.Pattern = v([^.]*)\..*
> v followed by any number of non '.' characters then followed by a '.' and
then any additional characters
Again that's not to say we shouldn't in the future fix this sort of thing,
but if we did we would try to take a more holistic view (perhaps trying to
regex and match any set of digits within the string rather than parsing them
separately as we do in the current code) and do it in another 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
