Copilot commented on code in PR #6595: URL: https://github.com/apache/hive/pull/6595#discussion_r3556956765
########## standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/index/store/IndexBackupUtils.java: ########## @@ -0,0 +1,121 @@ +/* + * 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.hive.search.index.store; + +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.util.List; +import java.util.Optional; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hive.search.config.IndexStateConfig; +import org.apache.hive.search.exception.IndexIOException; +import org.apache.hive.search.index.manifest.IndexManifest; + +public final class IndexBackupUtils { + private IndexBackupUtils() {} + + /** Push local index state to the backup (remote) location. */ + public static boolean syncToBackup(IndexStateClient local, IndexStateClient remote) + throws IOException { + Optional<IndexManifest> localManifest = local.readManifest(); + if (localManifest.isEmpty()) { + return false; + } + Optional<IndexManifest> remoteManifest = remote.readManifest(); + if (remoteManifest.isPresent() + && remoteManifest.get().lastEventId() >= localManifest.get().lastEventId()) { + return false; + } + applyDiff(local, remote, localManifest.get().diff(remoteManifest.orElse(null))); + return remote.writeManifest(localManifest.get()); + } + + /** + * If a previous restore left a staging manifest, finalize it when files already match, + * or keep it in place so {@link #restoreFromBackup} can resume. + */ + public static void resolveInterruptedRestore(IndexStateClient local) + throws IOException { + Optional<IndexManifest> staging = local.readStagingManifest(); + if (staging.isEmpty()) { + local.clearStagingManifest(); + return; + } Review Comment: resolveInterruptedRestore() clears the staging manifest when readStagingManifest() returns Optional.empty(). LocalStateClient.readStagingManifest() also returns empty on read/parsing failure, so this can delete a real staging manifest and prevent restoreFromBackup() from resuming after a partial/corrupted staging read. ########## standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/search/HybridSearch.java: ########## @@ -0,0 +1,187 @@ +/* + * 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.hive.search.search; + +import java.util.List; +import java.util.Map; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hive.search.config.SearchConfig; +import org.apache.hive.search.exception.SearchException; +import org.apache.hive.search.mapping.FieldSchema; +import org.apache.hive.search.mapping.IndexMapping; +import org.apache.hive.search.metastore.MetastoreTableMapper; + +/** Builds RRF queries that fuse lexical and semantic retrieval on hybrid fields. */ +public final class HybridSearch { + private HybridSearch() {} + + public record ParsedHybridQuery( + String field, + String queryText, + Float semanticWeight) {} + + public static ParsedHybridQuery parse(Object hybridBody, IndexMapping mapping) + throws SearchException { + if (hybridBody instanceof String queryText) { + String field = requireHybridField(mapping, null); + return new ParsedHybridQuery(field, queryText, null); + } + if (!(hybridBody instanceof Map<?, ?> body)) { + throw new SearchException("hybrid query must be a string or object"); + } + @SuppressWarnings("unchecked") + Map<String, Object> hybridMap = (Map<String, Object>) body; + + HybridWeights weights = parseHybridWeights(hybridMap); Review Comment: HybridSearch.parseHybridWeights() can throw IllegalArgumentException for invalid weight values, but HybridSearch.parse() only declares SearchException. This leaks an unchecked exception out through SearchInternal.compileHybridQuery(), bypassing the expected SearchException path and likely surfacing as an unexpected runtime failure instead of a user-facing validation error. ########## standalone-metastore/metastore-search/src/main/java/org/apache/lucene/search/package-info.java: ########## @@ -0,0 +1,21 @@ +/* + * 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. + */ + +/** + * Classes in this package are copied from Lucene codebase. + */ +package org.apache.lucene.search; Review Comment: Introducing new classes under the upstream Lucene package (org.apache.lucene.search) creates a split-package with the Lucene dependency and increases the risk of classpath/module-path conflicts or future Lucene name collisions. Prefer placing copied/adapted code under an org.apache.hive.* namespace (or shading/relocating) and referencing it from there. ########## standalone-metastore/metastore-search/src/main/java/org/apache/hive/search/mapping/field/Field.java: ########## @@ -0,0 +1,23 @@ +/* + * 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.hive.search.mapping.field; + +/** A single field value within a {@lin}. */ Review Comment: Javadoc link tag is misspelled ("{@lin}") which will render incorrectly and may trigger Javadoc warnings. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
