You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by twalthr <gi...@git.apache.org> on 2017/01/06 13:01:45 UTC

[GitHub] flink pull request #3073: [FLINK-5418] [table] Estimated row size does not s...

GitHub user twalthr opened a pull request:

    https://github.com/apache/flink/pull/3073

    [FLINK-5418] [table] Estimated row size does not support nested types

    This adds an estimate for array, composite and generic types.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/twalthr/flink FLINK-5418

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/3073.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 #3073
    
----
commit 8472588f35d95c2d227a3387090d3a6e6ded343d
Author: twalthr <tw...@apache.org>
Date:   2017-01-06T12:59:38Z

    [FLINK-5418] [table] Estimated row size does not support nested types

----


---
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] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    I will update this PR with the code from #3039 as a fix for the 1.2 release branch. What do you think is a good estimate for an array size? 16 elements? How do we want to estimate the size of a generic 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.
---

[GitHub] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    Merging this...


---
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] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    Good question, 16 sounds OK to me. For generic types I don't know, maybe 32 bytes?


---
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] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by twalthr <gi...@git.apache.org>.
Github user twalthr commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    I have chosen 128 now. Because generic types should be very expensive. If they have 4-8 fields with arrays and strings we should already be in that region.


---
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] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    Hi @twalthr, I have merged PR #3039 yesterday. Can you update this PR? 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] flink pull request #3073: [FLINK-5418] [table] Estimated row size does not s...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/3073


---
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] flink pull request #3073: [FLINK-5418] [table] Estimated row size does not s...

Posted by KurtYoung <gi...@git.apache.org>.
Github user KurtYoung commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3073#discussion_r95437867
  
    --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/plan/nodes/FlinkRel.scala ---
    @@ -120,6 +120,7 @@ trait FlinkRel {
             case typeName if SqlTypeName.YEAR_INTERVAL_TYPES.contains(typeName) => s + 8
             case typeName if SqlTypeName.DAY_INTERVAL_TYPES.contains(typeName) => s + 4
             case SqlTypeName.TIME | SqlTypeName.TIMESTAMP | SqlTypeName.DATE => s + 12
    +        case SqlTypeName.ARRAY | SqlTypeName.ROW | SqlTypeName.ANY => s + 24
    --- End diff --
    
    Any chance we can expand the ARRAY or the ROW type, and only counting the basic data structures to make this a little more accurate?


---
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] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    A better estimate for `ROW` will be merged with #3039. I think the best we can do for array is to estimate the size of one array element and have some default array size.


---
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] flink issue #3073: [FLINK-5418] [table] Estimated row size does not support ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/3073
  
    +1 to merge


---
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.
---