You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/09/09 20:17:22 UTC
[GitHub] [incubator-pinot] snleee opened a new pull request #5994: Add null check while fetching the schema
snleee opened a new pull request #5994:
URL: https://github.com/apache/incubator-pinot/pull/5994
When call ZKMetadataProvider:getSchema(), input parameter schemaName
is declared as `Nonnull`; however, it's possible that we pass null value.
This pr adds the null check to avoid the such case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5994: Add null check while fetching the schema
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5994:
URL: https://github.com/apache/incubator-pinot/pull/5994#discussion_r485899681
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metadata/ZKMetadataProvider.java
##########
@@ -274,14 +274,20 @@ public static Schema getTableSchema(@Nonnull ZkHelixPropertyStore<ZNRecord> prop
if (tableType == null || tableType == TableType.REALTIME) {
TableConfig realtimeTableConfig = getRealtimeTableConfig(propertyStore, tableName);
if (realtimeTableConfig != null) {
- schema = getSchema(propertyStore, realtimeTableConfig.getValidationConfig().getSchemaName());
+ String realtimeSchemaNameFromValidationConfig = realtimeTableConfig.getValidationConfig().getSchemaName();
+ if (realtimeSchemaNameFromValidationConfig != null) {
+ schema = getSchema(propertyStore, realtimeTableConfig.getValidationConfig().getSchemaName());
Review comment:
schema = getSchema(propertyStore, realtimeSchemaNameFromValidationConfig);
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on pull request #5994: Add null check while fetching the schema
Posted by GitBox <gi...@apache.org>.
snleee commented on pull request #5994:
URL: https://github.com/apache/incubator-pinot/pull/5994#issuecomment-689799046
Exception example:
```
2020/09/09 19:07:04.121 ERROR [ZKMetadataProvider] [grizzly-http-server-28] [pinot-controller] [] Caught exception while getting schema: null
java.lang.IllegalArgumentException: Path must not end with / character
at org.apache.helix.util.PathUtils.validatePath(PathUtils.java:57) ~[helix-core-0.9.4.1.jar:0.9.4.1]
at org.apache.helix.manager.zk.ZkCacheBaseDataAccessor.prependChroot(ZkCacheBaseDataAccessor.java:150) ~[helix-core-0.9.4.1.jar:0.9.4.1]
at org.apache.helix.manager.zk.ZkCacheBaseDataAccessor.get(ZkCacheBaseDataAccessor.java:355) ~[helix-core-0.9.4.1.jar:0.9.4.1]
at org.apache.helix.store.zk.AutoFallbackPropertyStore.get(AutoFallbackPropertyStore.java:101) ~[helix-core-0.9.4.1.jar:0.9.4.1]
at org.apache.pinot.common.metadata.ZKMetadataProvider.getSchema(ZKMetadataProvider.java:244) ~[pinot-common-0.5.0-dev-18617.jar:0.5.0-dev-18617-a31c06a9f77314234190cdd77d4657bc373d4e89]
at org.apache.pinot.common.metadata.ZKMetadataProvider.getTableSchema(ZKMetadataProvider.java:284) ~[pinot-common-0.5.0-dev-18617.jar:0.5.0-dev-18617-a31c06a9f77314234190cdd77d4657bc373d4e89]
at org.apache.pinot.controller.helix.core.PinotHelixResourceManager.getTableSchema(PinotHelixResourceManager.java:1048) ~[pinot-controller-0.5.0-dev-18617.jar:0.5.0-dev-18617-a31c06a9f77314234190cdd77d4657bc373d4e89]
at org.apache.pinot.controller.api.resources.PinotTableSchema.getTableSchema(PinotTableSchema.java:54) ~[pinot-controller-0.5.0-dev-18617.jar:?]
at jdk.internal.reflect.GeneratedMethodAccessor239.invoke(Unknown Source) ~[?:?]
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory.lambda$static$0(ResourceMethodInvocationHandlerFactory.java:52) ~[jersey-server-2.28.jar:?]
at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:124) [jersey-server-2.28.jar:?]
at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:167) [jersey-server-2.28.jar:?]
at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$TypeOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:219) [jersey-server-2.28.jar:?]
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5994: Add null check while fetching the schema
Posted by GitBox <gi...@apache.org>.
snleee commented on a change in pull request #5994:
URL: https://github.com/apache/incubator-pinot/pull/5994#discussion_r485901738
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metadata/ZKMetadataProvider.java
##########
@@ -274,14 +274,20 @@ public static Schema getTableSchema(@Nonnull ZkHelixPropertyStore<ZNRecord> prop
if (tableType == null || tableType == TableType.REALTIME) {
TableConfig realtimeTableConfig = getRealtimeTableConfig(propertyStore, tableName);
if (realtimeTableConfig != null) {
- schema = getSchema(propertyStore, realtimeTableConfig.getValidationConfig().getSchemaName());
+ String realtimeSchemaNameFromValidationConfig = realtimeTableConfig.getValidationConfig().getSchemaName();
+ if (realtimeSchemaNameFromValidationConfig != null) {
+ schema = getSchema(propertyStore, realtimeTableConfig.getValidationConfig().getSchemaName());
+ }
}
}
// Try to fetch offline schema if realtime schema does not exist
if (schema == null && (tableType == null || tableType == TableType.OFFLINE)) {
TableConfig offlineTableConfig = getOfflineTableConfig(propertyStore, tableName);
if (offlineTableConfig != null) {
- schema = getSchema(propertyStore, offlineTableConfig.getValidationConfig().getSchemaName());
+ String offlineSchemaNameFromValidationConfig = offlineTableConfig.getValidationConfig().getSchemaName();
+ if (offlineSchemaNameFromValidationConfig != null) {
+ schema = getSchema(propertyStore, offlineTableConfig.getValidationConfig().getSchemaName());
Review comment:
good catch :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5994: Add null check while fetching the schema
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5994:
URL: https://github.com/apache/incubator-pinot/pull/5994#discussion_r485899781
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/metadata/ZKMetadataProvider.java
##########
@@ -274,14 +274,20 @@ public static Schema getTableSchema(@Nonnull ZkHelixPropertyStore<ZNRecord> prop
if (tableType == null || tableType == TableType.REALTIME) {
TableConfig realtimeTableConfig = getRealtimeTableConfig(propertyStore, tableName);
if (realtimeTableConfig != null) {
- schema = getSchema(propertyStore, realtimeTableConfig.getValidationConfig().getSchemaName());
+ String realtimeSchemaNameFromValidationConfig = realtimeTableConfig.getValidationConfig().getSchemaName();
+ if (realtimeSchemaNameFromValidationConfig != null) {
+ schema = getSchema(propertyStore, realtimeTableConfig.getValidationConfig().getSchemaName());
+ }
}
}
// Try to fetch offline schema if realtime schema does not exist
if (schema == null && (tableType == null || tableType == TableType.OFFLINE)) {
TableConfig offlineTableConfig = getOfflineTableConfig(propertyStore, tableName);
if (offlineTableConfig != null) {
- schema = getSchema(propertyStore, offlineTableConfig.getValidationConfig().getSchemaName());
+ String offlineSchemaNameFromValidationConfig = offlineTableConfig.getValidationConfig().getSchemaName();
+ if (offlineSchemaNameFromValidationConfig != null) {
+ schema = getSchema(propertyStore, offlineTableConfig.getValidationConfig().getSchemaName());
Review comment:
schema = getSchema(propertyStore, offlineSchemaNameFromValidationConfig);
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] snleee merged pull request #5994: Add null check while fetching the schema
Posted by GitBox <gi...@apache.org>.
snleee merged pull request #5994:
URL: https://github.com/apache/incubator-pinot/pull/5994
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org