Lucene - Core
  1. Lucene - Core
  2. LUCENE-1836

Flexible QueryParser fails with local different from en_US

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.9
    • Fix Version/s: 2.9
    • Component/s: modules/other
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      I get the following error during the mentioned testcases on my computer, if I use the Locale de_DE (windows 32):

          [junit] Testsuite: org.apache.lucene.queryParser.standard.TestQPHelper
          [junit] Tests run: 29, Failures: 1, Errors: 0, Time elapsed: 1,156 sec
          [junit]
          [junit] ------------- Standard Output ---------------
          [junit] Result: (fieldX:xxxxx fieldy:xxxxxxxx)^2.0
          [junit] ------------- ---------------- ---------------
          [junit] Testcase: testLocalDateFormat(org.apache.lucene.queryParser.standard.TestQPHelper): FAILED
          [junit] expected:<1> but was:<0>
          [junit] junit.framework.AssertionFailedError: expected:<1> but was:<0>
          [junit]     at org.apache.lucene.queryParser.standard.TestQPHelper.assertHits(TestQPHelper.java:1148)
          [junit]     at org.apache.lucene.queryParser.standard.TestQPHelper.testLocalDateFormat(TestQPHelper.java:1005)
          [junit]     at org.apache.lucene.util.LuceneTestCase.runTest(LuceneTestCase.java:201)
          [junit]
          [junit]
          [junit] Test org.apache.lucene.queryParser.standard.TestQPHelper FAILED
          [junit] Testsuite: org.apache.lucene.queryParser.standard.TestQueryParserWrapper
          [junit] Tests run: 27, Failures: 1, Errors: 0, Time elapsed: 1,219 sec
          [junit]
          [junit] ------------- Standard Output ---------------
          [junit] Result: (fieldX:xxxxx fieldy:xxxxxxxx)^2.0
          [junit] ------------- ---------------- ---------------
          [junit] Testcase: testLocalDateFormat(org.apache.lucene.queryParser.standard.TestQueryParserWrapper):       FAILED
          [junit] expected:<1> but was:<0>
          [junit] junit.framework.AssertionFailedError: expected:<1> but was:<0>
          [junit]     at org.apache.lucene.queryParser.standard.TestQueryParserWrapper.assertHits(TestQueryParserWrapper.java:1120)
          [junit]     at org.apache.lucene.queryParser.standard.TestQueryParserWrapper.testLocalDateFormat(TestQueryParserWrapper.java:985)
          [junit]     at org.apache.lucene.util.LuceneTestCase.runTest(LuceneTestCase.java:201)
          [junit]
          [junit]
          [junit] Test org.apache.lucene.queryParser.standard.TestQueryParserWrapper FAILED
      

      With en_US as locale it works.

      1. LUCENE-1836.patch
        0.8 kB
        Robert Muir
      2. LUCENE-1836.patch
        1 kB
        Robert Muir
      3. LUCENE-1836.patch
        6 kB
        Robert Muir
      4. LUCENE-1836.patch
        9 kB
        Robert Muir
      5. LUCENE-1836.patch
        9 kB
        Uwe Schindler
      6. LUCENE-1836.patch
        9 kB
        Uwe Schindler

        Issue Links

          Activity

          Hide
          Luis Alves added a comment -

          I could also commit it, if you like. I only wanted to ask before commit, because it is assigned to you.

          I only realized I could have asked you, after I asked Michael. Next time

          Show
          Luis Alves added a comment - I could also commit it, if you like. I only wanted to ask before commit, because it is assigned to you. I only realized I could have asked you, after I asked Michael. Next time
          Hide
          Michael Busch added a comment -

          Committed revision 807494.

          (with the missing "abstract")

          Show
          Michael Busch added a comment - Committed revision 807494. (with the missing "abstract")
          Hide
          Uwe Schindler added a comment -

          OK! If you also have the missing "abstract" in it, go!

          Show
          Uwe Schindler added a comment - OK! If you also have the missing "abstract" in it, go!
          Hide
          Michael Busch added a comment -

          I just reviewed the patch and was going to commit it now, Uwe. OK?

          Show
          Michael Busch added a comment - I just reviewed the patch and was going to commit it now, Uwe. OK?
          Hide
          Uwe Schindler added a comment -

          Here the patch again with the missing "abstract".

          I could also commit it, if you like. I only wanted to ask before commit, because it is assigned to you. After that we can make progress with the other localization problems.

          Show
          Uwe Schindler added a comment - Here the patch again with the missing "abstract". I could also commit it, if you like. I only wanted to ask before commit, because it is assigned to you. After that we can make progress with the other localization problems.
          Hide
          Luis Alves added a comment -

          the test superclass should be abstract, everything else looks good! How to proceed?

          Hi Uwe,
          I reviewed the patch, looks good. We should commit it.
          We should open another JIRA issue with the remaining issues.

          Michael Bush do you have time to review and commit Uwe patch?

          Show
          Luis Alves added a comment - the test superclass should be abstract, everything else looks good! How to proceed? Hi Uwe, I reviewed the patch, looks good. We should commit it. We should open another JIRA issue with the remaining issues. Michael Bush do you have time to review and commit Uwe patch?
          Hide
          Uwe Schindler added a comment -

          the test superclass should be abstract, everything else looks good! How to proceed?

          Show
          Uwe Schindler added a comment - the test superclass should be abstract, everything else looks good! How to proceed?
          Hide
          Uwe Schindler added a comment -

          I modified the patch a little bit to only test locale-related tests (numbers, dates, farsi) with different default locales. This improved the time for the test by factor 3. I also reordered tearDown() to be reverse.

          Show
          Uwe Schindler added a comment - I modified the patch a little bit to only test locale-related tests (numbers, dates, farsi) with different default locales. This improved the time for the test by factor 3. I also reordered tearDown() to be reverse.
          Hide
          Uwe Schindler added a comment -

          Should I assign to this issue and commit the testcase and the fix of the tests?

          Show
          Uwe Schindler added a comment - Should I assign to this issue and commit the testcase and the fix of the tests?
          Hide
          Robert Muir added a comment -

          Uwe, ok. after this LocalizedTestCase is added, if you think it is a good idea, I can try to help resolve the same issues in the very similar test for the queryparser in core under a different issue.

          in my opinion 'ant test' should pass regardless of whether you happen to be on a Korean or Thai or German computer.

          Show
          Robert Muir added a comment - Uwe, ok. after this LocalizedTestCase is added, if you think it is a good idea, I can try to help resolve the same issues in the very similar test for the queryparser in core under a different issue. in my opinion 'ant test' should pass regardless of whether you happen to be on a Korean or Thai or German computer.
          Hide
          Uwe Schindler added a comment -

          The general contract in lucene should be that all date/time types in lucene should be normalized to UTC and stored like that. Query parsing should be done in the locale specified by the user and should be handled out of Lucene.

          The latest patch is ok and we should go forward with it.

          Show
          Uwe Schindler added a comment - The general contract in lucene should be that all date/time types in lucene should be normalized to UTC and stored like that. Query parsing should be done in the locale specified by the user and should be handled out of Lucene. The latest patch is ok and we should go forward with it.
          Hide
          Robert Muir added a comment - - edited

          Uwe, i undid my change for curiousity : the issue was not DateTools, but i still believe it was an overflow

          or the other more likely possibility is since the date in the test (2002 in Thai calendar) is really 1459 in the gregorian calendar, you get negative time

          java.lang.RuntimeException: time '-16122135600000' is too early, must be >= 0
          	at org.apache.lucene.document.DateField.timeToString(DateField.java:95)
          	at org.apache.lucene.document.DateField.dateToString(DateField.java:86)
          	at org.apache.lucene.queryParser.standard.TestQPHelper.getLegacyDate(TestQPHelper.java:633)
          	at org.apache.lucene.queryParser.standard.TestQPHelper.testLegacyDateRange(TestQPHelper.java:683)
          	...
          

          But I agree, we should be consistent about use of GregorianCalendar and I think the patch is correct in that sense.

          Show
          Robert Muir added a comment - - edited Uwe, i undid my change for curiousity : the issue was not DateTools, but i still believe it was an overflow or the other more likely possibility is since the date in the test (2002 in Thai calendar) is really 1459 in the gregorian calendar, you get negative time java.lang.RuntimeException: time '-16122135600000' is too early, must be >= 0 at org.apache.lucene.document.DateField.timeToString(DateField.java:95) at org.apache.lucene.document.DateField.dateToString(DateField.java:86) at org.apache.lucene.queryParser.standard.TestQPHelper.getLegacyDate(TestQPHelper.java:633) at org.apache.lucene.queryParser.standard.TestQPHelper.testLegacyDateRange(TestQPHelper.java:683) ... But I agree, we should be consistent about use of GregorianCalendar and I think the patch is correct in that sense.
          Hide
          Robert Muir added a comment -

          Uwe hmm, well then I am not sure. There was definitely a failure due to negative milliseconds values (and only with Thai calendar), so I assumed overflow.

          But, I do not remember if the failure was in DateTools, or DateField (the tests use both)

          Show
          Robert Muir added a comment - Uwe hmm, well then I am not sure. There was definitely a failure due to negative milliseconds values (and only with Thai calendar), so I assumed overflow. But, I do not remember if the failure was in DateTools, or DateField (the tests use both)
          Hide
          Uwe Schindler added a comment -

          The Buddhist calendar is internally a subclass of GregorianCalendar with some offsets... Interesting. http://www.docjar.com/html/api/sun/util/BuddhistCalendar.java.html

          The Java calendar will not overflow in 2038, as it uses a long (but the unix timestamp multiplied with 1000L), so it should last for several thousand/million years (I tested this), especially as the unix epoch is still always the same point in time (1970-01-01 expressed in the GregorianCalendar)

          The problem with DateTools is more, that the values stored in index are not correctly normalized, so the index would not work on another locale (especially as parsing the dates may fail, because other number of days per month). All date/times with DateTools should be indexed as normalized values (gregorian with GMT/UTC TZ) to achieve interoperability. The same like indexing date/time with NumericField using the ms since unix epoch.

          Show
          Uwe Schindler added a comment - The Buddhist calendar is internally a subclass of GregorianCalendar with some offsets... Interesting. http://www.docjar.com/html/api/sun/util/BuddhistCalendar.java.html The Java calendar will not overflow in 2038, as it uses a long (but the unix timestamp multiplied with 1000L), so it should last for several thousand/million years (I tested this), especially as the unix epoch is still always the same point in time (1970-01-01 expressed in the GregorianCalendar) The problem with DateTools is more, that the values stored in index are not correctly normalized, so the index would not work on another locale (especially as parsing the dates may fail, because other number of days per month). All date/times with DateTools should be indexed as normalized values (gregorian with GMT/UTC TZ) to achieve interoperability. The same like indexing date/time with NumericField using the ms since unix epoch.
          Hide
          Robert Muir added a comment -

          Uwe, yes. I think because Buddhist years are > 2038 now and it will cause integer overflow

          Show
          Robert Muir added a comment - Uwe, yes. I think because Buddhist years are > 2038 now and it will cause integer overflow
          Hide
          Uwe Schindler added a comment -

          Aah, so the Thai locale would break DateTools before my patch, as it would use the wrong calendar to format the dates stored in index. Thanks for the info!

          Show
          Uwe Schindler added a comment - Aah, so the Thai locale would break DateTools before my patch, as it would use the wrong calendar to format the dates stored in index. Thanks for the info!
          Hide
          Robert Muir added a comment -

          Uwe, no. The thai problem was that Calendar.getInstance() i think returns a BuddhistCalendar.
          This would give some negative milliseconds for a date component in the tests.

          so this is why i changed it to use GregorianCalendar... it is still locale-sensitive (formatted differently according to locale).
          But the test will not use calendar systems that really cannot be converted 1-1 with the gregorian system.

          Show
          Robert Muir added a comment - Uwe, no. The thai problem was that Calendar.getInstance() i think returns a BuddhistCalendar. This would give some negative milliseconds for a date component in the tests. so this is why i changed it to use GregorianCalendar... it is still locale-sensitive (formatted differently according to locale). But the test will not use calendar systems that really cannot be converted 1-1 with the gregorian system.
          Hide
          Uwe Schindler added a comment -

          Looks fine. So the Thai problem was also a whitespace problem?

          Show
          Uwe Schindler added a comment - Looks fine. So the Thai problem was also a whitespace problem?
          Hide
          Robert Muir added a comment -

          all tests pass under every locale with this patch...

          Show
          Robert Muir added a comment - all tests pass under every locale with this patch...
          Hide
          Robert Muir added a comment -

          Uwe, it is up to you. i thought it might be considered all the same problem, but it doesn't matter.

          if we change the test as Adriano suggests, it fixes Korean problem.
          But the tests do not pass under Thai locale for a different reason.

          Show
          Robert Muir added a comment - Uwe, it is up to you. i thought it might be considered all the same problem, but it doesn't matter. if we change the test as Adriano suggests, it fixes Korean problem. But the tests do not pass under Thai locale for a different reason.
          Hide
          Uwe Schindler added a comment -

          Should we at least first commit the patch for ParametricRangeQueryNodeProcessor that fixes the error for standard locales without spaces in dates? I think tis one is separate.

          Show
          Uwe Schindler added a comment - Should we at least first commit the patch for ParametricRangeQueryNodeProcessor that fixes the error for standard locales without spaces in dates? I think tis one is separate.
          Hide
          Adriano Crestani added a comment -

          If you double quote the values the syntax parser exits find: [ "02. 2. 1" TO "02. 2. 4"]

          Maybe, whoever formats the values should check if the values need to be double quoted or not, and double quote the formatted value when necessary.

          Show
          Adriano Crestani added a comment - If you double quote the values the syntax parser exits find: [ "02. 2. 1" TO "02. 2. 4"] Maybe, whoever formats the values should check if the values need to be double quoted or not, and double quote the formatted value when necessary.
          Hide
          Robert Muir added a comment -

          Adriano, also as I noted in LUCENE-1846, the old queryparser in core has this same issue.

          So if you are able to figure out an improvement to the javacc grammar to fix this, I think we should consider applying it there as well.

          Show
          Robert Muir added a comment - Adriano, also as I noted in LUCENE-1846 , the old queryparser in core has this same issue. So if you are able to figure out an improvement to the javacc grammar to fix this, I think we should consider applying it there as well.
          Hide
          Robert Muir added a comment -

          I don't see any need to escape whitespaces, as I said, once the syntax parser finds '[' or '

          Unknown macro: {' it should treat any sequence of chars as the value until it finds the token ' TO '. So, the only token needed to be escaped is 'TO' for the first range value, and ']' and '}

          ' for the second range value.

          Adriano, even better. This would be more user-friendly. I attached what i did so far, let me know if I can help in any other way!

          Show
          Robert Muir added a comment - I don't see any need to escape whitespaces, as I said, once the syntax parser finds '[' or ' Unknown macro: {' it should treat any sequence of chars as the value until it finds the token ' TO '. So, the only token needed to be escaped is 'TO' for the first range value, and ']' and '} ' for the second range value. Adriano, even better. This would be more user-friendly. I attached what i did so far, let me know if I can help in any other way!
          Hide
          Robert Muir added a comment -

          attached is the fix for german, but also the utility base class "LocalizedTestCase".

          I changed TestQPHelper to extend this in the patch, so it will run tests under every locale, including the failing Korean.

          Show
          Robert Muir added a comment - attached is the fix for german, but also the utility base class "LocalizedTestCase". I changed TestQPHelper to extend this in the patch, so it will run tests under every locale, including the failing Korean.
          Hide
          Adriano Crestani added a comment -

          Thanks for the quick response!

          (I would think requiring escaped whitespace in this situation would be acceptable?)

          I don't see any need to escape whitespaces, as I said, once the syntax parser finds '[' or '

          {' it should treat any sequence of chars as the value until it finds the token ' TO '. So, the only token needed to be escaped is 'TO' for the first range value, and ']' and '}

          ' for the second range value.

          Show
          Adriano Crestani added a comment - Thanks for the quick response! (I would think requiring escaped whitespace in this situation would be acceptable?) I don't see any need to escape whitespaces, as I said, once the syntax parser finds '[' or ' {' it should treat any sequence of chars as the value until it finds the token ' TO '. So, the only token needed to be escaped is 'TO' for the first range value, and ']' and '} ' for the second range value.
          Hide
          Robert Muir added a comment -

          Adriano, I can upload a specific patch for the korean issue tomorrow to make it easier.
          (i also want to see how old queryparser deals with this)

          the last patch has the fix to ParametricRange for the german issue.
          it also modifies the test in setUp() to run under the german locale to demonstrate it is fixed:

          Locale.setDefault(Locale.GERMANY);

          to see the separate korean problem, change the line of code in setUp() from Locale.GERMANY to Locale.KOREAN

          I tried escaping the whitespace with "\", but this didn't seem to resolve the problem.
          (I would think requiring escaped whitespace in this situation would be acceptable?)

          Show
          Robert Muir added a comment - Adriano, I can upload a specific patch for the korean issue tomorrow to make it easier. (i also want to see how old queryparser deals with this) the last patch has the fix to ParametricRange for the german issue. it also modifies the test in setUp() to run under the german locale to demonstrate it is fixed: Locale.setDefault(Locale.GERMANY); to see the separate korean problem, change the line of code in setUp() from Locale.GERMANY to Locale.KOREAN I tried escaping the whitespace with "\", but this didn't seem to resolve the problem. (I would think requiring escaped whitespace in this situation would be acceptable?)
          Hide
          Adriano Crestani added a comment -

          Hi,

          I'm not being able to reproduce the problem, I tried to apply both patches and I got no error, it passes fine on my eclipse env. Am I missing something?

          The issue is that under the korean locale, the query cannot be parsed.
          I believe the problem is that korean date format for this date (2/1/2002) look like "02. 2. 1"
          so the qp gives parsing errors, cannot parse ranges like [ 02. 2. 1 TO 02. 2. 4]

          Yes, it seems a syntax limitation, the values cannot contain whitespace, if there is any whitespace it expects the "TO" token. So it's a syntax limitation, but I see no reason to remove it and allow whitespaces in the range values, considering anything between '[' or '

          {' to ' TO ' and between ' TO ' to ']' or '}

          ' as the range values, example: [<anyt_sequence_of_chars> TO <any_sequence_of_chars>]

          Thoughts?

          Show
          Adriano Crestani added a comment - Hi, I'm not being able to reproduce the problem, I tried to apply both patches and I got no error, it passes fine on my eclipse env. Am I missing something? The issue is that under the korean locale, the query cannot be parsed. I believe the problem is that korean date format for this date (2/1/2002) look like "02. 2. 1" so the qp gives parsing errors, cannot parse ranges like [ 02. 2. 1 TO 02. 2. 4] Yes, it seems a syntax limitation, the values cannot contain whitespace, if there is any whitespace it expects the "TO" token. So it's a syntax limitation, but I see no reason to remove it and allow whitespaces in the range values, considering anything between '[' or ' {' to ' TO ' and between ' TO ' to ']' or '} ' as the range values, example: [<anyt_sequence_of_chars> TO <any_sequence_of_chars>] Thoughts?
          Hide
          Robert Muir added a comment -

          I played with this korean issue some, I am not sure what the fix should be (I am not too terribly familiar with the new qp, so that doesn't help).

          The issue is that under the korean locale, the query cannot be parsed.
          I believe the problem is that korean date format for this date (2/1/2002) look like "02. 2. 1"
          so the qp gives parsing errors, cannot parse ranges like [ 02. 2. 1 TO 02. 2. 4]

          Guessing this is due to whitespace present in korean date format?
          Set your locale in setUp() to Locale.setDefault(Locale.KOREA) to see the problem...

          Show
          Robert Muir added a comment - I played with this korean issue some, I am not sure what the fix should be (I am not too terribly familiar with the new qp, so that doesn't help). The issue is that under the korean locale, the query cannot be parsed. I believe the problem is that korean date format for this date (2/1/2002) look like "02. 2. 1" so the qp gives parsing errors, cannot parse ranges like [ 02. 2. 1 TO 02. 2. 4] Guessing this is due to whitespace present in korean date format? Set your locale in setUp() to Locale.setDefault(Locale.KOREA) to see the problem...
          Hide
          Robert Muir added a comment -

          fyi, I set this up, and found:

          Test failure of testLegacyDateRange occurred under a different Locale ko
          Test failure of testDateRange occurred under a different Locale ko
          Test failure of testLocalDateFormat occurred under a different Locale ja_JP_JP

          So i don't think its completely fixed yet, will look at the rest of these and upload a new patch (using this LocalizedTestCase setup)

          Show
          Robert Muir added a comment - fyi, I set this up, and found: Test failure of testLegacyDateRange occurred under a different Locale ko Test failure of testDateRange occurred under a different Locale ko Test failure of testLocalDateFormat occurred under a different Locale ja_JP_JP So i don't think its completely fixed yet, will look at the rest of these and upload a new patch (using this LocalizedTestCase setup)
          Hide
          Robert Muir added a comment -

          Uwe, yes only a few parts of lucene are sensitive to locales.

          So I think the ones that are, they can subclass LocalizedTestCase or whatever.

          But additionally I think in the future it would be nice if running ALL tests under a few different locales was part of the release process (basically what you have done here to find this issue!)

          Show
          Robert Muir added a comment - Uwe, yes only a few parts of lucene are sensitive to locales. So I think the ones that are, they can subclass LocalizedTestCase or whatever. But additionally I think in the future it would be nice if running ALL tests under a few different locales was part of the release process (basically what you have done here to find this issue!)
          Hide
          Uwe Schindler added a comment -

          Uwe, if you get a chance maybe you can confirm this fix to ParametricRange takes care of your problem?

          I applied your patch (without the setting of the default locale), so only the ParametricRange part and the test now passes. I think, this was it. Thanks!

          One thought i had on the test problem was to do something like you did for LUCENE-1825, make a base LocalizedTestCase or whatever that runs all tests with different default Locales.

          Good idea! In my last patch for 1825 I also added a Set, that can be specified in the tests ctor to limit the multiple runs only to specific testmethods. See TestIndexWriter in LUCENE-1843 (which takes very long, so I only tested some methods two times). Running really all tests about 50 times (number of locales in a standard jvm) would take too long I would only subclass local-aware tests and only a subset of testmethods, if there are too many long-time ones.

          Show
          Uwe Schindler added a comment - Uwe, if you get a chance maybe you can confirm this fix to ParametricRange takes care of your problem? I applied your patch (without the setting of the default locale), so only the ParametricRange part and the test now passes. I think, this was it. Thanks! One thought i had on the test problem was to do something like you did for LUCENE-1825 , make a base LocalizedTestCase or whatever that runs all tests with different default Locales. Good idea! In my last patch for 1825 I also added a Set, that can be specified in the tests ctor to limit the multiple runs only to specific testmethods. See TestIndexWriter in LUCENE-1843 (which takes very long, so I only tested some methods two times). Running really all tests about 50 times (number of locales in a standard jvm) would take too long I would only subclass local-aware tests and only a subset of testmethods, if there are too many long-time ones.
          Hide
          Robert Muir added a comment -

          Uwe, if you get a chance maybe you can confirm this fix to ParametricRange takes care of your problem?

          One thought i had on the test problem was to do something like you did for LUCENE-1825,
          make a base LocalizedTestCase or whatever that runs all tests with different default Locales.

          Show
          Robert Muir added a comment - Uwe, if you get a chance maybe you can confirm this fix to ParametricRange takes care of your problem? One thought i had on the test problem was to do something like you did for LUCENE-1825 , make a base LocalizedTestCase or whatever that runs all tests with different default Locales.
          Hide
          Robert Muir added a comment -

          when i say all the tests, i want to clarify, not limited to queryparser.

          what is some analyzer in contrib reads an input file with default platform encoding and gets strange results?

          In my opinion, we should not make queryparser tests complicated when in reality it would be better to consider other options, setting up hudson to test under different locales or something?
          I will read up on others strategies for things like this.

          Show
          Robert Muir added a comment - when i say all the tests, i want to clarify, not limited to queryparser. what is some analyzer in contrib reads an input file with default platform encoding and gets strange results? In my opinion, we should not make queryparser tests complicated when in reality it would be better to consider other options, setting up hudson to test under different locales or something? I will read up on others strategies for things like this.
          Hide
          Robert Muir added a comment -

          i believe Uwe has found a real bug here.
          take a look at my patch, in setUp() i set the default locale to German to duplicate his environment.
          without the change to ParametricRangeQuery, the test fails.

          if this looks ok, I will then think of a (clean) way to modify the tests so we don't have a future problem.

          unfortunately to really test it nice, I think you need to change the default locale as I did here... this is the only way I can think of to catch bugs like this.
          setting it to a random available locale doesn't seem to be best either, i would rather run all the tests under each locale!

          Show
          Robert Muir added a comment - i believe Uwe has found a real bug here. take a look at my patch, in setUp() i set the default locale to German to duplicate his environment. without the change to ParametricRangeQuery, the test fails. if this looks ok, I will then think of a (clean) way to modify the tests so we don't have a future problem. unfortunately to really test it nice, I think you need to change the default locale as I did here... this is the only way I can think of to catch bugs like this. setting it to a random available locale doesn't seem to be best either, i would rather run all the tests under each locale!
          Hide
          Robert Muir added a comment -

          Luis, no problem. I will take a look at this later today.

          Show
          Robert Muir added a comment - Luis, no problem. I will take a look at this later today.
          Hide
          Luis Alves added a comment -

          one last comment, I also think instead of depending upon the current Locale for the tests, we should iterate thru all (or at least some) available Locale's the jvm supports, and test each. This way, the tests test the localization, but will not fail depending upon the computer being used.

          +1 I really think that is much better testcase

          I'll see if I can look into this today.

          Robert when you have time to write that testcase,
          can you upload the patch, I would like to give it a try.

          Show
          Luis Alves added a comment - one last comment, I also think instead of depending upon the current Locale for the tests, we should iterate thru all (or at least some) available Locale's the jvm supports, and test each. This way, the tests test the localization, but will not fail depending upon the computer being used. +1 I really think that is much better testcase I'll see if I can look into this today. Robert when you have time to write that testcase, can you upload the patch, I would like to give it a try.
          Hide
          Luis Alves added a comment -

          linking

          Show
          Luis Alves added a comment - linking
          Hide
          Robert Muir added a comment -

          one last comment, I also think instead of depending upon the current Locale for the tests, we should iterate thru all (or at least some) available Locale's the jvm supports, and test each.

          Locale.getAvailableLocales()

          This way, the tests test the localization, but will not fail depending upon the computer being used.

          Show
          Robert Muir added a comment - one last comment, I also think instead of depending upon the current Locale for the tests, we should iterate thru all (or at least some) available Locale's the jvm supports, and test each. Locale.getAvailableLocales() This way, the tests test the localization, but will not fail depending upon the computer being used.
          Hide
          Robert Muir added a comment -

          Uwe, I saw several localization issues in the tests.
          The best way is to make sure things such as Locales and Collators are being set to the queryparser in all the test utility methods.

          Also, I think any hardcoded string dates should be converted to the local Locale before being parsed, rather than using Locale.US
          This way, its really testing the localization.

          I can look at this later if no one beats me to it, sorry I have to run for a while.

          Show
          Robert Muir added a comment - Uwe, I saw several localization issues in the tests. The best way is to make sure things such as Locales and Collators are being set to the queryparser in all the test utility methods. Also, I think any hardcoded string dates should be converted to the local Locale before being parsed, rather than using Locale.US This way, its really testing the localization. I can look at this later if no one beats me to it, sorry I have to run for a while.
          Hide
          Uwe Schindler added a comment -

          I tried the same last night, did not work. I also tried to set the timezone to GMT, no change. Maybe its a bug in DateField?

          Show
          Uwe Schindler added a comment - I tried the same last night, did not work. I also tried to set the timezone to GMT, no change. Maybe its a bug in DateField?
          Hide
          Robert Muir added a comment -

          gotta run off to a meeting, Uwe, I wonder if you could test the attached patch?

          Show
          Robert Muir added a comment - gotta run off to a meeting, Uwe, I wonder if you could test the attached patch?

            People

            • Assignee:
              Michael Busch
              Reporter:
              Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development