You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by PivotalSarge <gi...@git.apache.org> on 2017/03/02 22:51:53 UTC
[GitHub] geode-native pull request #44: GEODE-2578: Remove 64 KiB limit on query stri...
GitHub user PivotalSarge opened a pull request:
https://github.com/apache/geode-native/pull/44
GEODE-2578: Remove 64 KiB limit on query strings.
- Remove artificial cap of 65535 for query string
length by using 32 bits for the length of query
strings in DataOutput::writeFullUTF().
- Rename parameter to TcrMessage::writeStringPart()
whose name is misleading due to copy-and-paste.
- Add three unit tests for query-related messages:
* testConstructorEXECUTECQ_MSG_TYPE
* testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE
* testConstructorEXECUTECQ_WITH_IR_MSG_TYPE
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2578
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode-native/pull/44.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #44
----
commit 542c6349c7194558f463732a96a0e32a216a17a3
Author: Sarge <md...@pivotal.io>
Date: 2017-03-02T22:45:37Z
GEODE-2578: Remove 64 KiB limit on query strings.
- Remove artificial cap of 65535 for query string
length by using 32 bits for the length of query
strings in DataOutput::writeFullUTF().
- Rename parameter to TcrMessage::writeStringPart()
whose name is misleading due to copy-and-paste.
- Add three unit tests for query-related messages:
* testConstructorEXECUTECQ_MSG_TYPE
* testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE
* testConstructorEXECUTECQ_WITH_IR_MSG_TYPE
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] geode-native issue #44: GEODE-2578: Remove 64 KiB limit
on query strings.
Posted by Michael William Dodge <md...@pivotal.io>.
They didn't for me before I committed the code. Let me try them again.
> On 6 Mar, 2017, at 08:03, pivotal-jbarrett <gi...@git.apache.org> wrote:
>
> Github user pivotal-jbarrett commented on the issue:
>
> https://github.com/apache/geode-native/pull/44
>
> @PivotalSarge looks like some unit tests are failing.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
[GitHub] geode-native issue #44: GEODE-2578: Remove 64 KiB limit on query strings.
Posted by pivotal-jbarrett <gi...@git.apache.org>.
Github user pivotal-jbarrett commented on the issue:
https://github.com/apache/geode-native/pull/44
@PivotalSarge looks like some unit tests are failing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #44: GEODE-2578: Remove 64 KiB limit on query strings.
Posted by PivotalSarge <gi...@git.apache.org>.
Github user PivotalSarge commented on the issue:
https://github.com/apache/geode-native/pull/44
Roger that. Expect a new PR forthwith...
Sarge
> On 7 Mar, 2017, at 08:10, Jacob Barrett <no...@github.com> wrote:
>
> @PivotalSarge <https://github.com/PivotalSarge> can you rebase your branch off of develop. We backed out all those C++11 changes and now they are showing up in your pull.
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <https://github.com/apache/geode-native/pull/44#issuecomment-284767886>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATlz4jShpxAMJQ6goU2ocGZqUWWhijVQks5rjYGPgaJpZM4MRo4O>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #44: GEODE-2578: Remove 64 KiB limit on query strings.
Posted by PivotalSarge <gi...@git.apache.org>.
Github user PivotalSarge commented on the issue:
https://github.com/apache/geode-native/pull/44
The unit tests failed on Linux but not other platforms. It turns out that DataOutput has some not-very-obvious (and apparently platform-specific) behavior around buffer management that results in the return values from getBufferLength() and getRemainingBufferLength() depending on the behavior of other unit tests that may precede them. Hence, the test of ginormous query strings was causing the return values of those methods to differ from what would be returned if that test was *not* run. So I reworked how the DataOutput cursor advancement unit tests work and created GEODE-2598 to understand and test how DataOutput's buffer management works.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native pull request #44: GEODE-2578: Remove 64 KiB limit on query stri...
Posted by PivotalSarge <gi...@git.apache.org>.
Github user PivotalSarge closed the pull request at:
https://github.com/apache/geode-native/pull/44
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode-native issue #44: GEODE-2578: Remove 64 KiB limit on query strings.
Posted by pivotal-jbarrett <gi...@git.apache.org>.
Github user pivotal-jbarrett commented on the issue:
https://github.com/apache/geode-native/pull/44
@PivotalSarge can you rebase your branch off of develop. We backed out all those C++11 changes and now they are showing up in your pull.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---