Yannick Welsch created LUCENE-10474:
---
Summary: Avoid throwing StackOverflowError when creating RegExp
Key: LUCENE-10474
URL: https://issues.apache.org/jira/browse/LUCENE-10474
Project: Lucene - Core
uschindler commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072224477
> I think the apparent demerit of this patch is exposing dictionary
internals as public interfaces (and kuromoji and nori depend on it).
This is also my biggest concern,
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072243144
I just wanted to explain my view about interfaces. From my perspective, I
don't think this adds complexities to interfaces.
As I wrote in the Jira issue, there are only two con
uschindler commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072254448
> Our current kuromoji/nori interfaces mix up "dictionary-lookup" and
"language-specific feature", and in theory - they should be decoupled as
original [mecab](https://github.c
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072259013
> Just have one morphological module with pluggable implementations.
**In theory**, we should have one analysis engine and kuromoji and nori
should be the language-specific
uschindler commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072261885
So lets merge this PR. At a later stage we should collect all those
refactored classes and put them in a separate module "analysis-morph".
--
This is an automated message fro
uschindler commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072263863
Maybe export `org.apache.lucene.analysis.morph` only to kuromoji and nori.
The classes inside there are *all* private? Only the actual implementations
should be visible?
--
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072278589
> Maybe export org.apache.lucene.analysis.morph only to kuromoji and nori.
First I wanted and tried to do so, but in order to export
"org.apache.lucene.analysis.morph" pac
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072278589
> Maybe export org.apache.lucene.analysis.morph only to kuromoji and nori.
First I wanted and tried to do so, but in order to export
"org.apache.lucene.analysis.mor
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072278589
> Maybe export org.apache.lucene.analysis.morph only to kuromoji and nori.
First I wanted and tried to do so, but in order to export
"org.apache.lucene.analysis.mor
[
https://issues.apache.org/jira/browse/LUCENE-10473?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17508728#comment-17508728
]
Adrien Grand commented on LUCENE-10473:
---
I had a quick look at this and I think t
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072278589
> Maybe export org.apache.lucene.analysis.morph only to kuromoji and nori.
First I wanted and tried to do so, but in order to export
"org.apache.lucene.analysis.mor
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072278589
> Maybe export org.apache.lucene.analysis.morph only to kuromoji and nori.
First I wanted and tried to do so, but in order to export
"org.apache.lucene.analysis.mor
[
https://issues.apache.org/jira/browse/LUCENE-10473?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17508775#comment-17508775
]
Robert Muir commented on LUCENE-10473:
--
+1 to remove the atLeast trap, let's make
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072400574
> For now, I saw compilation fails with warning when I add to clause to
exports.
I temporarily suppressed this warning by `@SuppressWarnings("module")`, then
the whole pro
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072400574
> For now, I saw compilation fails with warning when I add to clause to
exports.
I temporarily suppressed this warning by `@SuppressWarnings("module")`, then
the wh
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072400574
> For now, I saw compilation fails with warning when I add to clause to
exports.
I temporarily suppressed this warning by `@SuppressWarnings("module")`, then
the wh
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072278589
> Maybe export org.apache.lucene.analysis.morph only to kuromoji and nori.
~First I wanted and tried to do so, but in order to export
"org.apache.lucene.analysis.mo
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072400574
> For now, I saw compilation fails with warning when I add to clause to
exports.
I temporarily suppressed this warning by `@SuppressWarnings("module")`, then
the wh
ywelsch opened a new pull request #752:
URL: https://github.com/apache/lucene/pull/752
Creating a regular expression using the RegExp class can easily result
in a StackOverflowError being thrown, for example when the input is
larger than the maximum stack depth. Throwing a StackOverflo
rmuir commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072449020
As a library, we should throw the correct exception type, we shouldn't
change it for fun. It is not correct to assume that this can only happen as
result of union either.
--
This
rmuir commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072455063
along the same lines of visibility, I think we actually hurt ourselves the
way we package things in kuromoji and nori. The unnecessary subpackages force a
lot of internal stuff to b
ywelsch commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072464227
> As a library, we should throw the correct exception type, we shouldn't
change it for fun. It is not correct to assume that this can only happen as
result of union either.
ywelsch commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072464227
> As a library, we should throw the correct exception type, we shouldn't
change it for fun. It is not correct to assume that this can only happen as
result of union either.
Yuti-G commented on a change in pull request #747:
URL: https://github.com/apache/lucene/pull/747#discussion_r829320811
##
File path: lucene/facet/src/java/org/apache/lucene/facet/Facets.java
##
@@ -48,4 +48,13 @@ public abstract FacetResult getTopChildren(int topN, String
dim
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1071100941
--
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 un
janhoy merged pull request #2647:
URL: https://github.com/apache/lucene-solr/pull/2647
--
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-unsu
gsmiller commented on a change in pull request #747:
URL: https://github.com/apache/lucene/pull/747#discussion_r829161071
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##
@@ -143,9 +146,49 @@ private FacetResult getPat
uschindler commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1071008332
--
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 unsubsc
rmuir commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072455063
along the same lines of visibility, I think we actually hurt ourselves the
way we package things in kuromoji and nori. The unnecessary subpackages force a
lot of internal stuff to b
uschindler edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1071008332
I like the idea to remove the code duplication and have only one
implementation.
On the other hand, if you look at LOC before/after: +1,818 −1,492
We now have 3
rmuir commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072449020
As a library, we should throw the correct exception type, we shouldn't
change it for fun. It is not correct to assume that this can only happen as
result of union either.
--
This
jpountz commented on pull request #672:
URL: https://github.com/apache/lucene/pull/672#issuecomment-1070993075
Thanks Robert!
--
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
mocobeta commented on a change in pull request #740:
URL: https://github.com/apache/lucene/pull/740#discussion_r829051989
##
File path:
lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/util/TokenInfoDictionaryEntryWriter.java
##
@@ -0,0 +1,221 @@
+/*
+ * License
rmuir commented on a change in pull request #740:
URL: https://github.com/apache/lucene/pull/740#discussion_r829016828
##
File path:
lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/util/TokenInfoDictionaryEntryWriter.java
##
@@ -0,0 +1,221 @@
+/*
+ * Licensed t
jpountz merged pull request #672:
URL: https://github.com/apache/lucene/pull/672
--
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..
javanna opened a new pull request #753:
URL: https://github.com/apache/lucene/pull/753
As part of #716 I moved the test to use a collector manager, but I forgot to
update one of the assertions.
We can't rely on totalHits being accurate when the search is executed my
multiple threads and
mikemccand commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072508590
Hmm maybe we could we preserve the full `StackOverflowException` as the
cause in the newly thrown `IllegalArgumentException`? I don't like
losing/suppressing that information
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1070917923
--
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 unsubscri
mikemccand commented on a change in pull request #751:
URL: https://github.com/apache/lucene/pull/751#discussion_r830102401
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/LongValueFacetCounts.java
##
@@ -348,6 +348,9 @@ private void increment(long value) {
@O
msokolov commented on pull request #738:
URL: https://github.com/apache/lucene/pull/738#issuecomment-1072526063
Thanks @vigyasharma , looks good.
--
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
msokolov commented on pull request #738:
URL: https://github.com/apache/lucene/pull/738#issuecomment-1072527486
Since a release candidate is out for testing, I'll wait until that process
completes before merging this. It would be safe anyway, and this is small, but
just in case there is so
rmuir commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072528358
I'm still -1 for the change. If you hit `StackOverFlowError`, really you
should let the VM exit. There are no guarantees at this point.
I don't care what OpenJDK does here, it
mikemccand commented on a change in pull request #633:
URL: https://github.com/apache/lucene/pull/633#discussion_r830125445
##
File path: lucene/core/src/java/org/apache/lucene/index/MergePolicy.java
##
@@ -567,6 +605,21 @@ public abstract MergeSpecification findMerges(
msokolov commented on pull request #752:
URL: https://github.com/apache/lucene/pull/752#issuecomment-1072535834
I agree with @rmuir - we should not be catching Error. The VM had to unwind
the stack and who knows where we are now. If we could somehow detect the
problem before it gets to tha
mikemccand commented on pull request #633:
URL: https://github.com/apache/lucene/pull/633#issuecomment-1072547237
> Is there a way to ensure all code paths in the random tests get executed?
I want to run the tests that invoke `addIndexes(CodecReader...)` for some
random flag value.
javanna commented on pull request #753:
URL: https://github.com/apache/lucene/pull/753#issuecomment-1072548142
yes that's an option too, thanks for the feedback, will do so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
javanna commented on pull request #753:
URL: https://github.com/apache/lucene/pull/753#issuecomment-1072550928
@jpountz I pushed an update
--
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 spe
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072579252
I looked at the packages again. It'd make sense to me to simply move all
classes in `.util` package (dictionary builder and writers) to `.dict` so that
classes in `.dict` can be
rmuir commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072597056
we could do that stuff in another PR. there is enough changes in this PR
already I think? And the problem is really a separate, existing problem...
--
This is an automated message
jpountz commented on a change in pull request #754:
URL: https://github.com/apache/lucene/pull/754#discussion_r830223881
##
File path:
lucene/core/src/java/org/apache/lucene/codecs/CompetitiveImpactAccumulator.java
##
@@ -151,9 +151,6 @@ public String toString() {
// Only
jpountz commented on pull request #754:
URL: https://github.com/apache/lucene/pull/754#issuecomment-1072649487
Here's the output from Gradle after running tests with `-Dtests.multiplier=3
-Dtests.nigthly=true`:
```
The slowest tests (exceeding 500 ms) during this run:
793.90s
[
https://issues.apache.org/jira/browse/LUCENE-10473?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17508980#comment-17508980
]
Adrien Grand commented on LUCENE-10473:
---
I opened https://github.com/apache/lucen
jpountz merged pull request #753:
URL: https://github.com/apache/lucene/pull/753
--
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..
[
https://issues.apache.org/jira/browse/LUCENE-10472?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17508982#comment-17508982
]
ASF subversion and git services commented on LUCENE-10472:
--
Co
[
https://issues.apache.org/jira/browse/LUCENE-10472?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17508984#comment-17508984
]
ASF subversion and git services commented on LUCENE-10472:
--
Co
[
https://issues.apache.org/jira/browse/LUCENE-10472?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17508983#comment-17508983
]
ASF subversion and git services commented on LUCENE-10472:
--
Co
[
https://issues.apache.org/jira/browse/LUCENE-10472?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Adrien Grand resolved LUCENE-10472.
---
Fix Version/s: 9.2
Resolution: Fixed
> TestMatchAllDocsQuery#testEarlyTermination fa
[
https://issues.apache.org/jira/browse/LUCENE-10204?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509054#comment-17509054
]
Marc D'Mello commented on LUCENE-10204:
---
I talked a bit about this with [~mikemcc
anshumg commented on pull request #2648:
URL: https://github.com/apache/lucene-solr/pull/2648#issuecomment-1072754453
Will also create corresponding PR(s) for Lucene and Solr repos if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please lo
uschindler commented on pull request #2648:
URL: https://github.com/apache/lucene-solr/pull/2648#issuecomment-1072758611
For custom versions you can normally set version.suffix property. That's
what we do for internal versions.
--
This is an automated message from the Apache Git Service.
anshumg commented on pull request #2648:
URL: https://github.com/apache/lucene-solr/pull/2648#issuecomment-1072777343
@uschindler - considering we already support `.1` and `.2` as the 4th part,
I think it makes sense to remove that check unless there's a reason why only
those 2 values sho
uschindler commented on pull request #2648:
URL: https://github.com/apache/lucene-solr/pull/2648#issuecomment-1072780086
The problem is that base version needs to be equal to the Version class in
Code. This version is also written into index format.
The .1 and .2 were special values
anshumg closed pull request #2648:
URL: https://github.com/apache/lucene-solr/pull/2648
--
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-uns
anshumg commented on pull request #2648:
URL: https://github.com/apache/lucene-solr/pull/2648#issuecomment-1072832459
@uschindler - the restriction now is to comply w/ the pattern that looks
like `baseVersion-suffix` i.e. have a `-` in the middle.
Do you think removing the requireme
jtibshirani commented on a change in pull request #736:
URL: https://github.com/apache/lucene/pull/736#discussion_r830381767
##
File path:
lucene/sandbox/src/java/org/apache/lucene/sandbox/search/IndexSortSortedNumericDocValuesRangeQuery.java
##
@@ -278,7 +287,21 @@ private Bo
[
https://issues.apache.org/jira/browse/LUCENE-9614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509091#comment-17509091
]
ASF subversion and git services commented on LUCENE-9614:
-
Commi
[
https://issues.apache.org/jira/browse/LUCENE-9614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509096#comment-17509096
]
ASF subversion and git services commented on LUCENE-9614:
-
Commi
[
https://issues.apache.org/jira/browse/LUCENE-9614?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509097#comment-17509097
]
ASF subversion and git services commented on LUCENE-9614:
-
Commi
anshumg opened a new pull request #2649:
URL: https://github.com/apache/lucene-solr/pull/2649
This may still not be ideal/acceptable but allows for having internal
version numbers in the `x.y.z.a` format instead of `x.y.z-a` format.
--
This is an automated message from the Apache Git Ser
anshumg commented on pull request #2649:
URL: https://github.com/apache/lucene-solr/pull/2649#issuecomment-1072869336
My concern here is for people who have scripts assuming the presence of `-`
automatically. This might break back-compat for those folks.
--
This is an automated message f
mocobeta commented on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072918490
I accidentally committed
https://github.com/apache/lucene/pull/740/commits/67ed0169733d3fcde47946f89dd6abac5fb31bce
and reverted it. It merges `.util` to `.dict` package, minimiz
Tomoko Uchida created LUCENE-10475:
--
Summary: Reconsider package structure in kuromoji and nori to
mininize classes' visibiilty
Key: LUCENE-10475
URL: https://issues.apache.org/jira/browse/LUCENE-10475
mocobeta edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1072918490
I accidentally committed
https://github.com/apache/lucene/pull/740/commits/67ed0169733d3fcde47946f89dd6abac5fb31bce
and reverted it. It merges `.util` to `.dict` package,
[
https://issues.apache.org/jira/browse/LUCENE-10475?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Tomoko Uchida updated LUCENE-10475:
---
Description:
Some internal dictionary classes in `.dict` package are exposed to public in
kkewwei closed pull request #741:
URL: https://github.com/apache/lucene/pull/741
--
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..
Yuti-G commented on a change in pull request #751:
URL: https://github.com/apache/lucene/pull/751#discussion_r830443213
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/LongValueFacetCounts.java
##
@@ -348,6 +348,9 @@ private void increment(long value) {
@Overr
[
https://issues.apache.org/jira/browse/LUCENE-10448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509171#comment-17509171
]
kkewwei commented on LUCENE-10448:
--
[~vigyas] I test again, and want to find any no-pa
[
https://issues.apache.org/jira/browse/LUCENE-10448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509171#comment-17509171
]
kkewwei edited comment on LUCENE-10448 at 3/19/22, 5:38 AM:
[
https://issues.apache.org/jira/browse/LUCENE-10448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17509171#comment-17509171
]
kkewwei edited comment on LUCENE-10448 at 3/19/22, 5:39 AM:
Yuti-G commented on a change in pull request #747:
URL: https://github.com/apache/lucene/pull/747#discussion_r830444104
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##
@@ -414,4 +505,101 @@ public int compare(FacetRes
Yuti-G commented on pull request #747:
URL: https://github.com/apache/lucene/pull/747#issuecomment-1072947749
> I like the progress. Thanks! Left some more detailed comments this time
around on the `SortedSetDocValuesFacetCounts` implementation. Thanks again!
Thanks @gsmiller so much
Yuti-G commented on a change in pull request #747:
URL: https://github.com/apache/lucene/pull/747#discussion_r829347283
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/sortedset/SortedSetDocValuesFacetCounts.java
##
@@ -414,4 +505,101 @@ public int compare(FacetRes
Yuti-G commented on a change in pull request #751:
URL: https://github.com/apache/lucene/pull/751#discussion_r830443213
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/LongValueFacetCounts.java
##
@@ -348,6 +348,9 @@ private void increment(long value) {
@Overr
Yuti-G commented on a change in pull request #751:
URL: https://github.com/apache/lucene/pull/751#discussion_r830443213
##
File path:
lucene/facet/src/java/org/apache/lucene/facet/LongValueFacetCounts.java
##
@@ -348,6 +348,9 @@ private void increment(long value) {
@Overr
85 matches
Mail list logo
