Skip to content
This repository has been archived by the owner on Jun 29, 2021. It is now read-only.

METAMODEL-1225 Add ColumnTypingStrategies and use them in CSV module. #240

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;

import org.apache.metamodel.schema.ColumnType;
Copy link
Contributor

Choose a reason for hiding this comment

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

The import org.apache.metamodel.schema.ColumnType is never used.

import org.apache.metamodel.schema.Table;

/**
* Defines the context for configuring the type for a single column in a
* {@link ColumnTypingStrategy} session.
*/
public interface ColumnTypingContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have both a ColumnTypingContext interface and a ColumnTypingContextImpl. In this case you have exactly one implementation of an interface, for which I don't expect you want to create different implementations in the future, so I would personally drop the interface and rename the implementation to ColumnTypingContext.


/**
* Gets the index of the column being configured.
*
* @return the column index
*/
int getColumnIndex();

/**
* Gets the {@link Table} that the column is to pertain to. If the table is
* not yet available then this may return null.
*
* @return the associated table
*/
Table getTable();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;

import org.apache.metamodel.schema.ColumnType;
Copy link
Contributor

Choose a reason for hiding this comment

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

The import org.apache.metamodel.schema.ColumnType is never used.

import org.apache.metamodel.schema.Table;


/**
* An implementation of {@link ColumnTypingContext} that holds necessary context about the column being configured.
*/
public class ColumnTypingContextImpl implements ColumnTypingContext {

private final int columnIndex;

private final Table table;


/**
* Creates a context to conifgure a column for a specific table.
* @param table The table that contains the column
* @param columnIndex the index in the table of the column being configured.
*/
public ColumnTypingContextImpl( Table table, int columnIndex ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is mostly a styling thing, but normally we don't put spaces after and before parentheses in constructor and method definitions and calls, also we try to promote making parameters (and variables) final when possible, so then this constructor definition would look like this:

    public ColumnTypingContextImpl(final Table table, final int columnIndex) {

Copy link
Author

Choose a reason for hiding this comment

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

I will go back and double check the space padding in the rest of the files. My work uses different formatting rules and I'm not used to handling multiple rulesets in my IDE.

I agree with making parameters final when possible. Does this also apply to parameters of lambda functions? for example an instance of the ColumnTypingSession after adding the default implementation to close:

        return (final ColumnTypingContext ctx) -> {
            if ( iterator.hasNext() ) {
                return iterator.next();
            }
            return null;
        };

or

        return ctx -> {
            if ( iterator.hasNext() ) {
                return iterator.next();
            }
            return null;
        };

Personally I treat parameters as final whether they are marked as such or not, and I don't see any final lambda parameters in the project so far.

Copy link
Contributor

Choose a reason for hiding this comment

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

You make a valid point, because with the second example you're able to assign a new Object (if you were able to instantiate ColumnTypingContext) to the ctx parameter. Even though I wouldn't consider doing that.

It's just that in practice I always see the second example (the one you implemented), so for lambdas let's not start adding finals (and types) to the formal parameters now.

this.table = table;
this.columnIndex = columnIndex;
}


/**
* Creates a context a column to be configured.
* @param columnIndex the index in the table of the column being configured.
*/
public ColumnTypingContextImpl( int columnIndex ) {
this( null, columnIndex );
}


@Override
public int getColumnIndex() {
return columnIndex;
}


@Override
public Table getTable() {
return table;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;


import org.apache.metamodel.schema.Column;
import org.apache.metamodel.schema.ColumnType;
import org.apache.metamodel.schema.Table;

import java.io.Closeable;

/**
* Represents a 'session' in which a single {@link Table}'s {@link Column}s are
* assigned {@link ColumnType}s.
*/
public interface ColumnTypingSession extends Closeable {

/**
* Provides the type to apply for a given column.
*
* @param ctx the context of the column naming taking place. This contains
* column index, intrinsic type etc. if available.
* @return the type to provide to the column.
*/
public ColumnType getNextColumnType( ColumnTypingContext ctx );

/**
* Ends the column typing session.
*/
@Override
void close();
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the close method is already defined on the Closeable interface, it's not necessary to define it here again. You could change this to a default implementation of the close method, i.e.:

    @Override
    default void close() {
    }

In this manner you wouldn't have to implement it on the anonymous inner classes returned by the startColumnTypingSession on the CustomColumnTypingStrategy and DefaultColumnTypingStrategy classes.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;

import org.apache.metamodel.schema.ColumnType;

import java.util.List;

/**
* Constructors and common utilities for {@link ColumnTypingStrategy} objects.
*/
public class ColumnTypingStrategies {

private static final DefaultColumnTypingStrategy DEFAULT_STRATEGY = new DefaultColumnTypingStrategy();


private ColumnTypingStrategies() {
}


/**
* The default strategy assumes all column types are text.
*
* @return a strategy that defaults to text
*/
public static ColumnTypingStrategy defaultStrategy() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Java method names are by convention preferably verbs, so then this should be getDefaultStrategy and the ones below should be getCustomStrategy.

return DEFAULT_STRATEGY;
}


/**
* Utility to create a {@link CustomColumnTypingStrategy}.
*
* @param columnTypes the types of each column
* @return a strategy for custom type configuration
*/
public static ColumnTypingStrategy customTypes( List<ColumnType> columnTypes ) {
return new CustomColumnTypingStrategy( columnTypes );
}


/**
* Utility to create a {@link CustomColumnTypingStrategy}.
*
* @param columnTypes the types of each column
* @return a strategy for custom type configuration
*/
public static ColumnTypingStrategy customTypes( ColumnType... columnTypes ) {
return new CustomColumnTypingStrategy( columnTypes );
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;


import java.io.Serializable;

/**
* A strategy that defines the data types of columns. Such strategies are
* mostly used when a particular datastore is not itself intrinsically
* specifying the column type.
*/
public interface ColumnTypingStrategy extends Serializable {

ColumnTypingSession startColumnTypingSession();

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;

import org.apache.metamodel.schema.ColumnType;
import org.apache.metamodel.util.SimpleTableDef;

import java.util.Arrays;
import java.util.Iterator;
import java.util.List;

/**
* A {@link ColumnTypingStrategy} that allows the user to supply his own list of column types
*/
public class CustomColumnTypingStrategy implements ColumnTypingStrategy {

private static final long serialVersionUID = 1L;

private final List<ColumnType> columnTypes;


/**
* Creates the strategy based on the provided types.
*
* @param columnTypes a list of column types to be applied to a table.
*/
public CustomColumnTypingStrategy( List<ColumnType> columnTypes ) {
kaspersorensen marked this conversation as resolved.
Show resolved Hide resolved
this.columnTypes = columnTypes;
}


/**
* Creates the strategy based on the provided types.
*
* @param columnTypes a list of column types to be applied to a table.
*/
public CustomColumnTypingStrategy( ColumnType... columnTypes ) {
this( Arrays.asList( columnTypes ) );
}

public CustomColumnTypingStrategy( SimpleTableDef tableDef ) {
this( Arrays.asList( tableDef.getColumnTypes() ) );
}


@Override
public ColumnTypingSession startColumnTypingSession() {
final Iterator<ColumnType> iterator = columnTypes.iterator();
return new ColumnTypingSession() {

@Override
public ColumnType getNextColumnType( ColumnTypingContext ctx ) {
if ( iterator.hasNext() ) {
return iterator.next();
}
return null;
}


@Override
public void close() {
}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.metamodel.schema.typing;

import org.apache.metamodel.schema.ColumnType;

public class DefaultColumnTypingStrategy implements ColumnTypingStrategy {

private static final long serialVersionUID = 1L;


public DefaultColumnTypingStrategy() {
}


@Override
public ColumnTypingSession startColumnTypingSession() {
return new ColumnTypingSession() {

@Override
public ColumnType getNextColumnType( ColumnTypingContext ctx ) {
return ColumnType.STRING;
}


@Override
public void close() {
}
};
}
}
Loading