Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-5188] Create analog of RowCsvInputFormat in java and adjust all the imports of Row and RowTypeInfo #3003

Closed
wants to merge 6 commits into from

Conversation

tonycox
Copy link
Contributor

@tonycox tonycox commented Dec 14, 2016

Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the How To Contribute guide.
In addition to going through the list, please provide a meaningful description of your changes.

  • General

    • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
    • The pull request addresses only one issue
    • Each commit in the PR has a meaningful commit message (including the JIRA id)
  • Documentation

    • Documentation has been added for new functionality
    • Old documentation affected by the pull request has been updated
    • JavaDoc for public methods has been added
  • Tests & Build

    • Functionality added by the pull request is covered by tests
    • mvn clean verify has been executed successfully locally or a Travis build has passed

@tonycox tonycox changed the title [Flink-5188] Create analog of RowCsvInputFormat in java and adjust all the imports of Row and RowTypeInfo [FLINK-5188] Create analog of RowCsvInputFormat in java and adjust all the imports of Row and RowTypeInfo Dec 14, 2016
@tonycox tonycox force-pushed the FLINK-5188 branch 2 times, most recently from 3317c2b to 0edfd48 Compare December 14, 2016 07:53
Copy link
Contributor

@fhueske fhueske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the doing this refactoring @tonycox!
I had a few comments but can fix them myself before merging.

Thanks, Fabian

import org.apache.flink.types.Row;
import org.apache.flink.types.parser.FieldParser;

@Internal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I we move it here, we can make this@PublicEvolving.

@@ -168,8 +168,10 @@ abstract class BatchTableEnvironment(
private[flink] def explain(table: Table, extended: Boolean): String = {
val ast = table.getRelNode
val optimizedPlan = optimize(ast)
val dataSet = translate[Row](optimizedPlan)(TypeExtractor.createTypeInfo(classOf[Row]))
dataSet.output(new DiscardingOutputFormat[Row])
val dataSet = translate[org.apache.flink.types.Row](optimizedPlan) (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an import for Row instead of using the full class name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see why you did it this way. I'll move this PR behind #3004.

@@ -327,7 +327,9 @@ abstract class StreamTableEnvironment(
def explain(table: Table): String = {
val ast = table.getRelNode
val optimizedPlan = optimize(ast)
val dataStream = translate[Row](optimizedPlan)(TypeExtractor.createTypeInfo(classOf[Row]))
val dataStream = translate[org.apache.flink.types.Row](optimizedPlan)(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add import for Row

@@ -28,6 +28,7 @@ import org.apache.flink.api.table.typeutils.RowComparator.{createAuxiliaryFields
import org.apache.flink.core.memory.{DataInputView, DataOutputView, MemorySegment}
import org.apache.flink.types.KeyFieldOutOfBoundsException


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary change


import scala.collection.JavaConversions._

object TypeConverter {

val DEFAULT_ROW_TYPE = new RowTypeInfo(Seq()).asInstanceOf[TypeInformation[Any]]
val DEFAULT_ROW_TYPE = new org.apache.flink.api.java.typeutils.RowTypeInfo(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import RowTypeInfo

@@ -166,5 +166,5 @@ class SqlExpressionTest extends ExpressionTestBase {
override def testData: Any = new Row(0)

override def typeInfo: TypeInformation[Any] =
new RowTypeInfo(Seq()).asInstanceOf[TypeInformation[Any]]
new RowTypeInfo(Seq.empty[TypeInformation[Any]]: _*).asInstanceOf[TypeInformation[Any]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be changed to new RowTypeInfo()?

@@ -117,14 +117,14 @@ abstract class ExpressionTestBase {
// compile and evaluate
val clazz = new TestCompiler[MapFunction[Any, String]]().compile(genFunc)
val mapper = clazz.newInstance()
val result = mapper.map(testData).asInstanceOf[Row]
val result = mapper.map(testData).asInstanceOf[org.apache.flink.types.Row]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import Row

import org.apache.flink.api.table.typeutils.RowComparatorTest.MyPojo
import org.apache.flink.api.java.typeutils.{RowTypeInfo => RowTypeInfoNew}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to rename here?

@@ -21,7 +21,8 @@ package org.apache.flink.api.table.typeutils
import org.apache.flink.api.common.ExecutionConfig
import org.apache.flink.api.common.typeinfo.{BasicTypeInfo, TypeInformation}
import org.apache.flink.api.common.typeutils.{ComparatorTestBase, TypeComparator, TypeSerializer}
import org.apache.flink.api.table.Row
import org.apache.flink.types.Row
import org.apache.flink.api.java.typeutils.{RowTypeInfo => RowTypeInfoNew}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why renaming here?

@@ -50,8 +50,8 @@ class RowSerializerTest {

@Test
def testRowSerializer(): Unit = {
val rowInfo: TypeInformation[Row] = new RowTypeInfo(
Seq(BasicTypeInfo.INT_TYPE_INFO, BasicTypeInfo.STRING_TYPE_INFO))
val rowInfo = new org.apache.flink.api.java.typeutils.RowTypeInfo(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import RowTypeInfo

@tonycox
Copy link
Contributor Author

tonycox commented Dec 14, 2016

Hi @fhueske okay, as you wish

fhueske pushed a commit to fhueske/flink that referenced this pull request Dec 14, 2016
…lls for new Row type.

- Port RowCsvInputFormat to Java and move it to flink-core.

This closes apache#3003.
@fhueske
Copy link
Contributor

fhueske commented Dec 15, 2016

Merging

@asfgit asfgit closed this in 4d27f8f Dec 15, 2016
joseprupi pushed a commit to joseprupi/flink that referenced this pull request Feb 12, 2017
…lls to new Row type.

- Port RowCsvInputFormat to Java and move it to flink-core.

This closes apache#3003.
hequn8128 pushed a commit to hequn8128/flink that referenced this pull request Jun 22, 2017
…lls to new Row type.

- Port RowCsvInputFormat to Java and move it to flink-core.

This closes apache#3003.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants