Github user pnowojski commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4587#discussion_r160397308
  
    --- Diff: 
flink-libraries/flink-cep/src/main/java/org/apache/flink/cep/pattern/AndFilterFunction.java
 ---
    @@ -1,55 +0,0 @@
    -/*
    - * 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.flink.cep.pattern;
    -
    -import org.apache.flink.api.common.functions.FilterFunction;
    -
    -/**
    - * A filter function which combines two filter functions with a logical 
and. Thus, the filter
    - * function only returns true, iff both filters return true.
    - *
    - * @param <T> Type of the element to filter
    - * @deprecated This is only used when migrating from an older Flink 
version.
    - * Use the {@link org.apache.flink.cep.pattern.conditions.AndCondition} 
instead.
    - */
    -@Deprecated
    -public class AndFilterFunction<T> implements FilterFunction<T> {
    --- End diff --
    
    Sorry
    > I'm kind of worried 
    
    I meant: "it seems to me like this change should be ok, the only thing that 
I'm not 100% sure". In other words I don't expect to have find any issues with 
this code basing on my understanding of it.
    
    The case and the test that I had in mind should make sure, that checkpoints 
created/taken in 1.3.x do not use those `*FilterFunction` classes. Correct me 
if I'm wrong, but I have an impression if 1.3.x somehow created a checkpoint 
using `*FilterFunction`(unlikely? impossible?), before your change such 
checkpoint would still correctly load. That's why I'm missing those `and`/`or` 
conditions in `CEPMigrationTest` from `1.3.x` -> `1.4+`. Also such test might 
be useful for the future.
    
    Maybe with some extra knowledge about the previous changes this code has 
undergone it's obvious that this code just works, but I'm missing this context 
(I'm new to the CEP stuff :( )
      


---

Reply via email to