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/05/15 07:09:38 UTC

[GitHub] flink pull request #3897: [FLINK-6517] [table] Support multiple consecutive ...

GitHub user twalthr opened a pull request:

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

    [FLINK-6517] [table] Support multiple consecutive windows

    This PR adds support for multiple consecutive windows for the Table API. SQL requires additional group auxiliary functions. I will open a followup issue for that.

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

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

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

    https://github.com/apache/flink/pull/3897.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 #3897
    
----
commit faa2646e73039f72f9849efe8eb36f0f2abb793a
Author: twalthr <tw...@apache.org>
Date:   2017-05-12T08:03:25Z

    [FLINK-6517] [table] Support multiple consecutive windows

----


---
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 #3897: [FLINK-6517] [table] Support multiple consecutive ...

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

    https://github.com/apache/flink/pull/3897#discussion_r116433771
  
    --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/windowProperties.scala ---
    @@ -39,20 +49,20 @@ abstract class WindowProperty(child: Expression) extends UnaryExpression {
           ValidationFailure("Child must be a window reference.")
         }
     
    -  private[flink] def toNamedWindowProperty(name: String)(implicit relBuilder: RelBuilder)
    +  def toNamedWindowProperty(name: String)
    --- End diff --
    
    define in one line?


---
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 #3897: [FLINK-6517] [table] Support multiple consecutive ...

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

    https://github.com/apache/flink/pull/3897#discussion_r116436281
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/calcite/RelTimeIndicatorConverterTest.scala ---
    @@ -28,7 +28,7 @@ import org.apache.flink.table.functions.TableFunction
     import org.apache.flink.table.plan.logical.TumblingGroupWindow
     import org.apache.flink.table.utils.TableTestBase
     import org.apache.flink.table.utils.TableTestUtil._
    -import org.junit.Test
    +import org.junit.{Ignore, Test}
    --- End diff --
    
    `Ignore` is unused


---
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 #3897: [FLINK-6517] [table] Support multiple consecutive windows

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

    https://github.com/apache/flink/pull/3897
  
    Thanks for the review @fhueske. I will merge 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 pull request #3897: [FLINK-6517] [table] Support multiple consecutive ...

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

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


---
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 #3897: [FLINK-6517] [table] Support multiple consecutive ...

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

    https://github.com/apache/flink/pull/3897#discussion_r116433350
  
    --- Diff: flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/fieldExpression.scala ---
    @@ -132,31 +134,61 @@ case class WindowReference(name: String) extends Attribute {
           throw new ValidationException("Cannot rename window reference.")
         }
       }
    +
    +  override def toString: String = s"'$name"
     }
     
     abstract class TimeAttribute(val expression: Expression)
       extends UnaryExpression
    -  with NamedExpression {
    +  with WindowProperty {
     
       override private[flink] def child: Expression = expression
    -
    -  override private[flink] def name: String = expression match {
    -    case UnresolvedFieldReference(name) => name
    -    case _ => throw new ValidationException("Unresolved field reference expected.")
    -  }
    -
    -  override private[flink] def toAttribute: Attribute =
    -    throw new UnsupportedOperationException("Time attribute can not be used solely.")
     }
     
     case class RowtimeAttribute(expr: Expression) extends TimeAttribute(expr) {
     
    -  override private[flink] def resultType: TypeInformation[_] =
    +  override private[flink] def validateInput(): ValidationResult = {
    +    child match {
    +      case WindowReference(_, Some(tpe)) if !isRowtimeIndicatorType(tpe) =>
    +        ValidationFailure("A proctime window can not guarantee a rowtime attribute.")
    --- End diff --
    
    -> "A proctime window cannot provide a rowtime attribute"?


---
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 #3897: [FLINK-6517] [table] Support multiple consecutive ...

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

    https://github.com/apache/flink/pull/3897#discussion_r116436437
  
    --- Diff: flink-libraries/flink-table/src/test/scala/org/apache/flink/table/runtime/datastream/TimeAttributesITCase.scala ---
    @@ -28,13 +28,13 @@ import org.apache.flink.streaming.api.watermark.Watermark
     import org.apache.flink.streaming.util.StreamingMultipleProgramsTestBase
     import org.apache.flink.table.api.scala._
     import org.apache.flink.table.api.scala.stream.utils.StreamITCase
    -import org.apache.flink.table.api.{TableEnvironment, TableException, Types}
    +import org.apache.flink.table.api.{TableEnvironment, TableException, Types, ValidationException}
     import org.apache.flink.table.calcite.RelTimeIndicatorConverterTest.TableFunc
     import org.apache.flink.table.expressions.TimeIntervalUnit
     import org.apache.flink.table.runtime.datastream.TimeAttributesITCase.TimestampWithEqualWatermark
     import org.apache.flink.types.Row
     import org.junit.Assert._
    -import org.junit.Test
    +import org.junit.{Ignore, Test}
    --- End diff --
    
    `Ignore` is unused


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