You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by kavinderd <gi...@git.apache.org> on 2016/07/21 16:28:46 UTC
[GitHub] incubator-hawq pull request #808: HAWQ-944. Implement new pg_ltoa function a...
GitHub user kavinderd opened a pull request:
https://github.com/apache/incubator-hawq/pull/808
HAWQ-944. Implement new pg_ltoa function as per postgres
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kavinderd/incubator-hawq HAWQ-944
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/808.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 #808
----
commit fb32032529eeec680d921383c056168910d99da9
Author: Kavinder Dhaliwal <ka...@gmail.com>
Date: 2016-07-20T23:14:10Z
HAWQ-944. Implement new pg_ltoa function as per postgres
----
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
Added `INT32_CHAR_SIZE` to `configure.in`
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by edespino <gi...@git.apache.org>.
Github user edespino commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
@kavinderd - Any progress on this issue? It is currently in a conflict situation as reported in the PR checks "This branch has conflicts that must be resolved". There doesn't appear to be any progress on the corresponding Jira as well (HAWQ-944) since last summer (July 2016).
---
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] incubator-hawq pull request #808: HAWQ-944. Implement new pg_ltoa function a...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/808#discussion_r71923516
--- Diff: src/backend/access/external/pxfutils.c ---
@@ -39,7 +39,7 @@ bool are_ips_equal(char *ip1, char *ip2)
/* override port str with given new port int */
void port_to_str(char **port, int new_port)
{
- char tmp[10];
+ char tmp[sizeof(new_port) + 2];
--- End diff --
This will not be sufficient. sizeof would return 4 and tmp would be allocated only 6 bytes which wouldn't be sufficient to store the 32 bit interger as a string
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
I would prefer if we have a general util function that determines precisely the number of bytes to "stringify" the given data type (int16 or int32) as a character array
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
+1
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
@kavinderd I will review this next Monday, sorry for the latency. If it is a hurry, you could at somebody else to review. Thanks!
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
@paul-guo- @xunzhang Please review, I think I covered all invocations of the two functions.
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by huor <gi...@git.apache.org>.
Github user huor commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
+1
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
@huor @paul-guo- Can you look at this PR when you get a chance. I need one more +1
---
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] incubator-hawq pull request #808: HAWQ-944. Implement new pg_ltoa function a...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-hawq/pull/808
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
I don't see INT32_CHAR_SIZE defined anywhere
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
I suspect pg_ltoa() will be faster if it fills the characters one by one in descending order, instead of current solution: filling in ascending order and then swapping.
Given the code comes from pg, I'd give a +1 at first.
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
It looks like pg_ltoa() does not work for the corner case: integer 0?
---
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] incubator-hawq issue #808: HAWQ-944. Implement new pg_ltoa function as per p...
Posted by kavinderd <gi...@git.apache.org>.
Github user kavinderd commented on the issue:
https://github.com/apache/incubator-hawq/pull/808
@xunzhang No rush, take your time
---
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.
---